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

Tracking Issue for -Z terminal-width #84673

Closed
ehuss opened this issue Apr 28, 2021 · 10 comments · Fixed by rust-lang/cargo#11494
Closed

Tracking Issue for -Z terminal-width #84673

ehuss opened this issue Apr 28, 2021 · 10 comments · Fixed by rust-lang/cargo#11494
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics

Comments

@ehuss
Copy link
Contributor

ehuss commented Apr 28, 2021

Status: The rustc side has been stabilized in 1.64 as the --diagnostic-width option. This tracking issue is still open for the cargo side of changes. See #84673 (comment).

This is a tracking issue for the -Z terminal-width flag. This flag tells rustc the width of the terminal so that it can truncate long lines in diagnostic output to the correct length. A corresponding flag of the same name is available in Cargo, which detects the terminal width and sends in the appropriate value to rustc.

Unresolved Questions

Unknown.

Implementation history

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

@ehuss ehuss added A-diagnostics Area: Messages for errors, warnings, and lints C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. WG-diagnostics Working group: Diagnostics labels Apr 28, 2021
@ehuss
Copy link
Contributor Author

ehuss commented Apr 28, 2021

cc @rust-lang/wg-diagnostics Hope you don't mind I open this. If anyone knows of any unresolved questions or blockers for this, I would appreciate if you could update the issue (or perhaps there is no desire to carry this forward?).

@estebank estebank added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 30, 2021
@shepmaster
Copy link
Member

I wanted this feature to be able to automatically format errors to a width suitable for inclusion on slides. However, it does not:

% rustc +nightly src/main.rs -Z terminal-width=80
error[E0425]: cannot find value `and_then_an_argument` in this scope
 --> src/main.rs:2:43
  |
2 |     a_very_long_function_name_that_pushes(and_then_an_argument);
  |                                           ^^^^^^^^^^^^^^^^^^^^ not found in this scope

this scope exceeds the specified width of 80 columns. I'd prefer it be

% rustc +nightly src/main.rs -Z terminal-width=80
error[E0425]: cannot find value `and_then_an_argument` in this scope
 --> src/main.rs:2:43
  |
2 |     a_very_long_function_name_that_pushes(and_then_an_argument);
  |                                           ^^^^^^^^^^^^^^^^^^^^ not found in
  |                                                                this scope

Should I register this as a separate issue?

@estebank
Copy link
Contributor

That should indeed be a separate issue, I think. It is related, but independent. I also know it will be very difficult to come up with a good layout algorithm :-/

For things like slides, I think a better option would be html output.

@davidtwco
Copy link
Member

davidtwco commented Apr 4, 2022

I think that we should go ahead with stabilizing this, it's a small compiler feature that hasn't seen any major changes since it was implemented, and is useful for people invoking rustc through build systems other than Cargo.

Stabilization PR: #95635

@rfcbot fcp merge

Brief summary

rustc currently detects the width of the terminal and will truncate diagnostic output when it includes source code lines which are longer than the terminal width.

error[E0308]: mismatched types
  --> $DIR/flag-human.rs:7:17
   |
LL | ..._: () = 42;
   |       --   ^^ expected `()`, found integer
   |       |
   |       expected due to this

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

However, rustc is unable to detect the terminal width when it is being invoked indirectly (i.e. through a build tool like Cargo). --terminal-width (currently -Zterminal-width) enables build tools to provide the terminal width to rustc so that it can continue to perform this truncation.

Implementation History

Test Coverage

FAQ

1. Is a flag necessary to turn this behaviour off?
--terminal-width doesn't do anything if it hasn't been explicitly provided to the compiler. When emitting an error and deciding whether to truncate output, rustc uses (in priority order):

  1. --terminal-width (if provided)
  2. If the compiler is being invoked for UI testing, then the terminal width is fixed at 140 columns.
  3. Using term_size to determine the current terminal width, and if that fails, defaulting to 140 columns.

Having this flag on stable won't change any behaviour, just allow tools to tell rustc the terminal width.

2. Can the terminal width be determined through environment variables?
Shells don't provide the terminal size to processes as an environment variable. There are "fake" environment variables available in the shell - namely COLUMNS and LINES - from some shells, but the shell wouldn't be able to keep these variables up to date if they were provided to a process, so it doesn't give them to the process at all. Processes can determine the terminal width by sending ioctl commands, asking stty size, or by listening for SIGWINCH signals. rustc uses the term_size crate to determine the terminal width when possible.

Outstanding bugs

Notes

  • If this is stabilized, then Cargo will need to be updated to use the stable flag, cc @ehuss

@rfcbot
Copy link

rfcbot commented Apr 4, 2022

Team member @davidtwco has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 4, 2022
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label May 19, 2022
@rfcbot
Copy link

rfcbot commented May 19, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label May 19, 2022
@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label May 29, 2022
@rfcbot
Copy link

rfcbot commented May 29, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 29, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 9, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 8, 2022
…ion, r=oli-obk

sess: stabilize `--terminal-width` as `--diagnostic-width`

Formerly `-Zterminal-width`, `--terminal-width` allows the user or build
tool to inform rustc of the width of the terminal so that diagnostics
can be truncated.

Pending agreement to stabilize, see tracking issue at rust-lang#84673.

r? `@oli-obk`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 8, 2022
…ion, r=oli-obk

sess: stabilize `--terminal-width` as `--diagnostic-width`

Formerly `-Zterminal-width`, `--terminal-width` allows the user or build
tool to inform rustc of the width of the terminal so that diagnostics
can be truncated.

Pending agreement to stabilize, see tracking issue at rust-lang#84673.

r? ``@oli-obk``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 8, 2022
…ion, r=oli-obk

sess: stabilize `--terminal-width` as `--diagnostic-width`

Formerly `-Zterminal-width`, `--terminal-width` allows the user or build
tool to inform rustc of the width of the terminal so that diagnostics
can be truncated.

Pending agreement to stabilize, see tracking issue at rust-lang#84673.

r? ```@oli-obk```
@davidtwco
Copy link
Member

Stabilization pull request has landed at #95635.

@ehuss
Copy link
Contributor Author

ehuss commented Jul 8, 2022

Thanks @davidtwco!

I posted rust-lang/cargo#10833 to fix Cargo's tests.

I'd like to discuss how this interface works in Cargo. I'm wondering if we should provide any sort of option to allow the user to force the width over the auto-detected version?

Currently Cargo has the term.progress.width config option to control the width used for the progress bar. I'm thinking of a few options:

  1. Don't allow the user to control the width.
  2. Use the term.progress.width option if it is set.
  3. Add a new term.terminal-width option. term.progress.width can use term.terminal-width if the progress width is not set (which kinda makes term.progress.width pointless, maybe it could be deprecated?).

I kinda lean towards option 3, but I dislike adding too many configuration options.

Does anyone have any thoughts? cc @rust-lang/cargo

bors added a commit to rust-lang/cargo that referenced this issue Jul 9, 2022
Update terminal-width flag.

The rustc flag `-Zterminal-width` has been stabilized as `--terminal-width` in rust-lang/rust#95635. This updates cargo to use the new flag so that tests will pass.

Tests won't pass until the next nightly is published in about 10 hours from now. I just wanted to post this to get ahead of the breaking change.

This doesn't stabilize in cargo because that will take more time, and this is needed to prevent CI from failing. Will continue the stabilization discussion at rust-lang/rust#84673.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants