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

add support for rustc's --error-format short #4720

Closed

Conversation

QuietMisdreavus
Copy link
Member

As implemented in rust-lang/rust#44636, this exposes the --error-format short in Cargo's --message-format flag.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@matklad
Copy link
Member

matklad commented Nov 13, 2017

implementation looks good to me!

One thing I am not certain about is that names human and short don't really fit together. Should we rename (with deprecation) human to long?

Am I correct that current human should be preserved as a default?

cc @rust-lang/cargo

@QuietMisdreavus
Copy link
Member Author

That makes sense, but may also require intervention in rustc as well, since human is also used there.

As for being default, human is still the default in rustc, so i codified that here. The Default impl was needed to bring it into BuildConfig, so its #[derive(Default)] would work.

@alexcrichton
Copy link
Member

I think this option to rustc was supposed to be unstable? If so I'm somewhat wary of landing this now...

@QuietMisdreavus
Copy link
Member Author

Since rust-lang/rust#46005 was merged, making that parameter unstable, should i just close this?

@alexcrichton
Copy link
Member

Ah yes, let's do that for now.

@matklad
Copy link
Member

matklad commented Nov 17, 2017

should i just close this?

Hm, I think it would be beneficial to expose this feature to Cargo users somehow: I am afraid rustc only option wouldn't get much testing without being able to enable it with Cargo. Or will it be possible to add RUST_FLAGS=["--message-format", "short"] to .cargo/config to achieve this effect? Will it work with tools which want to pass --message-format=json?

@alexcrichton
Copy link
Member

We could consider adding an unstable cargo option?

@pickfire
Copy link
Contributor

pickfire commented Aug 6, 2018

Any update on this after rust-lang/rust#49546 was merged?

bors added a commit that referenced this pull request Aug 12, 2018
Add support for rustc's --error-format short

Running a local build of this branch on some broken code shows this kind of output:

    18:42:29 $ dcargo check --message-format=short --tests
        Checking bufstream v0.1.3
        Checking cargo v0.30.0 (file:///d/cargo)
    tests/testsuite/config.rs:298:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:307:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:363:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:367:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:371:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:375:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:382:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:386:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:400:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:428:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:479:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:491:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:496:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:501:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:506:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:512:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:582:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:660:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:666:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:672:9: error[E0308]: mismatched types
    tests/testsuite/config.rs:678:9: error[E0308]: mismatched types
    error: aborting due to 21 previous errors
    error: Could not compile `cargo`.
    warning: build failed, waiting for other jobs to finish...
    error: build failed

Rehash of @QuietMisdreavus' #4720 now that `--short-message` is stable (thanks for the ping @pickfire!).

Feedback welcome.
@sanmai-NL
Copy link

sanmai-NL commented Sep 6, 2018

@matklad: ping, see preceding post. Seems like low-hanging fruit? Since #5879 it’s supported with at least the check and doc subcommands it seems, but with run nor check it works yet with cargo 1.29.0-nightly (0ec7281b9 2018-08-20):

error: Found argument '--error-format' which wasn't expected, or isn't valid in this context

USAGE:
    cargo run [OPTIONS] [--] [args]..

Update: Argh, they named the option differently between rustc and cargo --error-format vs. --message-format.

On the bright side, this issue can be closed.

@pickfire
Copy link
Contributor

pickfire commented Sep 7, 2018

Maybe we should change the arguments to be the same?

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.

6 participants