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.

Sign up for GitHub

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

Jump to bottom

Fix #128930: Print documentation of CLI options missing their arg #128961

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Fix #128930: Print documentation of CLI options missing their arg #128961

merged 2 commits into from
Sep 17, 2024

Conversation

GKFX
Copy link
Contributor

@GKFX GKFX commented Aug 11, 2024

Fix #128930. Failing to give an argument to CLI options which require it now prints something like:

$ rustc --print
error: Argument to option 'print' missing
       Usage:
           --print [crate-name|file-names|sysroot|target-libdir|cfg|check-cfg|calling-conventions|target-list|target-cpus|target-features|relocation-models|code-models|tls-models|target-spec-json|all-target-specs-json|native-static-libs|stack-protector-strategies|link-args|deployment-target]
                               Compiler information to print on stdout

@rustbot
Copy link
Collaborator

rustbot commented Aug 11, 2024

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 11, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GKFX
Copy link
Contributor Author

GKFX commented Aug 27, 2024

r? compiler

@rustbot rustbot assigned BoxyUwU and unassigned TaKO8Ki Aug 27, 2024
@workingjubilee
Copy link
Member

workingjubilee commented Sep 16, 2024 •
edited
Loading

@GKFX Why does this have a different error output than https://github.com/rust-lang/rust/blob/ae75a9b33aef4bdd03f33bc75bdf44fe3a48356d/tests/ui/invalid-compile-flags/print.stderr exactly? I mean, I'd expect the first line to be different, of course.

@Noratrieb
Copy link
Member

because that is a special case for --print, instead of generic code like here

@jieyouxu jieyouxu assigned jieyouxu and unassigned BoxyUwU Sep 17, 2024
jieyouxu
jieyouxu approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks. AFAICT the changes are reasonable. We can always adjust the exact message or its exact emission logic in the future, but this looks like a good ux improvement.

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 17, 2024

📌 Commit ae75a9b has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2024
@workingjubilee
Copy link
Member

workingjubilee commented Sep 17, 2024 •
edited
Loading

because that is a special case for --print, instead of generic code like here

Aha, I see. Then that makes sense.

fee1-dead added a commit to fee1-dead-contrib/rust that referenced this pull request Sep 17, 2024
@fee1-dead
Rollup merge of rust-lang#128961 - GKFX:issue-128930-explain-missing-…
9b91d67 
…option, r=jieyouxu

Fix rust-lang#128930: Print documentation of CLI options missing their arg

Fix rust-lang#128930. Failing to give an argument to CLI options which require it now prints something like:
```
$ rustc --print
error: Argument to option 'print' missing
       Usage:
           --print [crate-name|file-names|sysroot|target-libdir|cfg|check-cfg|calling-conventions|target-list|target-cpus|target-features|relocation-models|code-models|tls-models|target-spec-json|all-target-specs-json|native-static-libs|stack-protector-strategies|link-args|deployment-target]
                               Compiler information to print on stdout
```
@fee1-dead fee1-dead mentioned this pull request Sep 17, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 17, 2024
@bors
Auto merge of rust-lang#130461 - fee1-dead-contrib:rollup-lbf8gqt, r=…
466c6fd 
…fee1-dead

Rollup of 8 pull requests

Successful merges:

 - rust-lang#128961 (Fix rust-lang#128930: Print documentation of CLI options missing their arg)
 - rust-lang#129073 (Relate receiver invariantly in method probe for `Mode::Path`)
 - rust-lang#129674 (Add new_cyclic_in for Rc and Arc)
 - rust-lang#130201 (Encode `coroutine_by_move_body_def_id` in crate metadata)
 - rust-lang#130275 (Don't call `extern_crate` when local crate name is the same as a dependency and we have a trait error)
 - rust-lang#130440 (Don't ICE in `opaque_hidden_inferred_bound` lint for RPITIT in trait with no default method body)
 - rust-lang#130454 (tests: allow trunc/select instructions to be missing)
 - rust-lang#130458 (`rustc_codegen_ssa` cleanups)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 17, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 17, 2024
@bors
Auto merge of rust-lang#130473 - matthiaskrgr:rollup-lf29374, r=matth…
f609b7e 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128535 (Format `std::env::consts` docstrings with markdown backticks)
 - rust-lang#128961 (Fix rust-lang#128930: Print documentation of CLI options missing their arg)
 - rust-lang#129988 (Use `Vec` in `rustc_interface::Config::locale_resources`)
 - rust-lang#130201 (Encode `coroutine_by_move_body_def_id` in crate metadata)
 - rust-lang#130275 (Don't call `extern_crate` when local crate name is the same as a dependency and we have a trait error)
 - rust-lang#130314 (Use the same precedence for all macro-like exprs)
 - rust-lang#130440 (Don't ICE in `opaque_hidden_inferred_bound` lint for RPITIT in trait with no default method body)
 - rust-lang#130458 (`rustc_codegen_ssa` cleanups)
 - rust-lang#130469 (Mark `where_clauses_object_safety` as removed)

r? `@ghost`
`@rustbot` modify labels: rollup
@GKFX
Copy link
Contributor Author

GKFX commented Sep 17, 2024

I don't think there was much flexibility available from getopts in how this appears. The error gives you a name, that name lets you find the RustcOptGroup, and the RustcOptGroup lets you populate a getopts::Options. The only way to get usage information out of that is by printing usage information in a format like this PR does. To get a different format, you'd want to either store the descriptions on the RustcOptGroup or make changes to getopts. To minimize the complexity of this change, I just printed the usage in the format getopts provides.

@bors bors merged commit fe3428d into rust-lang:master Sep 17, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 17, 2024
@rust-timer
Unrolled build for rust-lang#128961
1ddb2ef 
Rollup merge of rust-lang#128961 - GKFX:issue-128930-explain-missing-option, r=jieyouxu

Fix rust-lang#128930: Print documentation of CLI options missing their arg

Fix rust-lang#128930. Failing to give an argument to CLI options which require it now prints something like:
```
$ rustc --print
error: Argument to option 'print' missing
       Usage:
           --print [crate-name|file-names|sysroot|target-libdir|cfg|check-cfg|calling-conventions|target-list|target-cpus|target-features|relocation-models|code-models|tls-models|target-spec-json|all-target-specs-json|native-static-libs|stack-protector-strategies|link-args|deployment-target]
                               Compiler information to print on stdout
```
@GKFX GKFX deleted the issue-128930-explain-missing-option branch September 18, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.83.0
Development

Successfully merging this pull request may close these issues.

rustc --print doesn't list valid options
9 participants
@GKFX @rustbot @rust-log-analyzer @workingjubilee @Noratrieb @jieyouxu @bors @BoxyUwU @TaKO8Ki