Skip to content
This repository was archived by the owner on Oct 10, 2023. It is now read-only.

Print underlying error when we fail to detect a project#121

Merged
cole-h merged 2 commits intomainfrom
cole/ds-294-running-riff-with---offline-on-a-new
Sep 9, 2022
Merged

Print underlying error when we fail to detect a project#121
cole-h merged 2 commits intomainfrom
cole/ds-294-running-riff-with---offline-on-a-new

Conversation

@cole-h
Copy link
Copy Markdown
Member

@cole-h cole-h commented Sep 8, 2022

e.g.:

image

image


I'm not set on the "red-ify all the errors", but I think it looks good.

One issue with the current approach is that we duplicate the error from

"'{}' does not contain a project recognized by Riff.",
(due to
`{colored_project_dir}` doesn't contain a project recognized by Riff.\n\
).

Could be addressed by slightly changing the wording so we get:

image

instead of

image

Suggestions welcome.

@cole-h cole-h requested a review from Hoverbear September 8, 2022 21:48
@cole-h cole-h force-pushed the cole/ds-294-running-riff-with---offline-on-a-new branch from 41b541f to a7f331a Compare September 8, 2022 22:03
nix = "nix".cyan(),
nix_install_url = "https://nixos.org/download.html".blue().underline(),
);
eprintln!("{err_msg}\n\nUnderlying error:\n{err}", err = err.red());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused since according to the docs we already would print that underlying error? https://docs.rs/eyre/0.6.8/eyre/struct.Report.html#display-representations

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the change here is that it is colored red (so as to stand out more, but I'm not married to the idea and can easily revert that commit).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no strong opinions about this. 😅

Copy link
Copy Markdown
Contributor

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THe code seems correct, though I'm not entirely sure this is a beneficial change.

@cole-h cole-h merged commit dbe2d59 into main Sep 9, 2022
@cole-h cole-h deleted the cole/ds-294-running-riff-with---offline-on-a-new branch September 9, 2022 17:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants