Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print something when nickel doc succeeds #1729

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

yannham
Copy link
Member

@yannham yannham commented Nov 30, 2023

Closues #1656.

The doc subcommand has been reported as confusing, because it generates file in a default location but just returns silently upon success. This PR adds a print statement that says where the document has been generated instead, and serves a confirmation that everything went well as well.

@github-actions github-actions bot temporarily deployed to pull request November 30, 2023 15:10 Inactive
cli/src/doc.rs Outdated
out_path
.as_ref()
.map(Cow::as_ref)
.unwrap_or(DEFAULT_OUT_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.unwrap_or(DEFAULT_OUT_DIR)
.expect("There should always be an output path")

Since we're always producing an output path if self.stdout is not set, I feel it would be better to panic here if no output path gets set. Just as an insurance policy against the path logic changing in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, but then I think I prefer to destructure the option instead of relying on the related-but-the-compiler-can't-prove-it boolean flag. That is, if out_path is Some, we print it, otherwise we don't print anything.

The doc subcommand has been reported to be a bit confusing, because it
generates file in a default location but just returns without further
details upon success. This commits add a print statement that says where
the document has been generated instead.
@github-actions github-actions bot temporarily deployed to pull request December 1, 2023 09:55 Inactive
@yannham yannham added this pull request to the merge queue Dec 1, 2023
Merged via the queue into master with commit 5c4298f Dec 1, 2023
5 checks passed
@yannham yannham deleted the fix/nickel-doc-show-out-loc branch December 1, 2023 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants