Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add option to anonymize line numbers#3

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
oli-obk merged 2 commits intorust-lang:masterfromphansch:anonymized_line_numbers
Jun 13, 2019

Conversation

@phansch
Copy link
Contributor

@phanschphansch commentedJun 9, 2019
edited
Loading

With the current diagnostics emitter of Rust, the-Z ui-testing flag
allows to 'anonymize' line numbers in the UI test output.

This means that a line such as:

 2 |     concat!(b'f');

is turned into:

LL |     concat!(b'f');

This is done because it makes the diff of UI test output changes much
less noisy.

To support this withannotate-snippet, we add a second parameter to
DisplayListFormatter that, when true, replaces the line numbers in the
left column with the textLL. The replacement text is alwaysLL and
it does not affect the initial location line number.

In the newannotate-snippet emitter in rustc, we can then use this
parameter depending on whether the-Z ui-testing flag is set or not.

Closes#2
ccrust-lang/rust#59346

@phansch
Copy link
ContributorAuthor

One of my small gripes with Rust is that it doesn't have keyword arguments (yet?). That would make passing these boolean options much more understandable at the call sites. I'm not sure if there's currently a clearer way?

@oli-obk
Copy link
Contributor

keyword arguments

you can always create structs with appropriate field names ;)

phansch reacted with thumbs up emoji

With the current diagnostics emitter of Rust, the `-Z ui-testing` flagallows to 'anonymize' line numbers in the UI test output.This means that a line such as:     2 |     concat!(b'f');is turned into:    LL |     concat!(b'f');This is done because it makes the diff of UI test output changes muchless noisy.To support this with `annotate-snippet`, we add a second parameter to`DisplayListFormatter` that, when true, replaces the line numbers in theleft column with the text `LL`. The replacement text is always `LL` andit does not affect the initial location line number.In the new `annotate-snippet` emitter in rustc, we can then use thisparameter depending on whether the `-Z ui-testing` flag is set or not.
@phanschphanschforce-pushed theanonymized_line_numbers branch from3d97fb9 tod6c36a6CompareJune 13, 2019 05:10
@phansch
Copy link
ContributorAuthor

phansch commentedJun 13, 2019
edited
Loading

Replaced the magic constant.

@zbraniecki what are your feelings regarding the change of the api of theDisplayListFormatter constructor? I feel like it's still fine when passing two booleans. If there turns out to be a need for more configuration, it could be changed later?

@zbraniecki
Copy link
Contributor

If there turns out to be a need for more configuration, it could be changed later?

Yep, it's an internal API, I'm fine with adjusting it later.

@oli-obk
Copy link
Contributor

I wonder...

@bors r+

@oli-obk
Copy link
Contributor

I guess not

@oli-obkoli-obk merged commitd62b321 intorust-lang:masterJun 13, 2019
@phanschphansch deleted the anonymized_line_numbers branchJune 13, 2019 08:58
@phansch
Copy link
ContributorAuthor

Thanks! I think this will also need a new release before it can be used in rustc, right?

@phansch
Copy link
ContributorAuthor

@zbraniecki (or someone else?) could you push a new version to crates.io?

zbraniecki reacted with thumbs up emoji

@zbraniecki
Copy link
Contributor

yeah, will do.

@zbraniecki
Copy link
Contributor

done. 0.6.0 released.

@zbraniecki
Copy link
Contributor

@phansch did you intend to turn also context lines (without numbers) toLL?

I see this:

error[E0003]: Expected token: "=" --> ../fluent-bundle/examples/resources/en-US/simple.ftl:2:18  |2 | input-parse-error Error: Could not parse input `{ $input }`. Reason: { $reason }  |                   ^^ Expected token: "="  |-----------------------------

turned into:

error[E0003]: Expected token: "="  --> ../fluent-bundle/examples/resources/en-US/simple.ftl:2:18LL |LL | input-parse-error Error: Could not parse input `{ $input }`. Reason: { $reason }LL |                   ^^ Expected token: "="LL |-----------------------------

I think I'd expect:

error[E0003]: Expected token: "="  --> ../fluent-bundle/examples/resources/en-US/simple.ftl:2:18   |LL | input-parse-error Error: Could not parse input `{ $input }`. Reason: { $reason }   |                   ^^ Expected token: "="   |-----------------------------

instead.

kennytm reacted with thumbs up emoji

@phansch
Copy link
ContributorAuthor

Yeah I noticed that, too. That's a bug of the current implementation. My current plan is to only showLL forDisplaySourceLine::Content and whereDisplayLine::Source has alineno set. I think that should be enough?

(I've been a bit time restricted the past month or so but I want to get back into it again this month)

phansch added a commit to phansch/rust that referenced this pull requestJul 25, 2019
This adds support for the `-Z ui-testing` flag to the newannotate-snippet diagnostic emitter.The support for the flag was added to `annotate-snippet-rs` in these PRs:*rust-lang/annotate-snippets-rs#3*rust-lang/annotate-snippets-rs#5Closesrust-lang#61811
Centril added a commit to Centril/rust that referenced this pull requestJul 26, 2019
…estebanklibrustc_errors: Support ui-testing flag in annotate-snippet emitterThis adds support for the `-Z ui-testing` flag to the newannotate-snippet diagnostic emitter.Support for the flag was added to `annotate-snippet-rs` in these PRs:*rust-lang/annotate-snippets-rs#3*rust-lang/annotate-snippets-rs#5r?@estebankClosesrust-lang#61811
Centril added a commit to Centril/rust that referenced this pull requestJul 26, 2019
…estebanklibrustc_errors: Support ui-testing flag in annotate-snippet emitterThis adds support for the `-Z ui-testing` flag to the newannotate-snippet diagnostic emitter.Support for the flag was added to `annotate-snippet-rs` in these PRs:*rust-lang/annotate-snippets-rs#3*rust-lang/annotate-snippets-rs#5r?@estebankClosesrust-lang#61811
Centril added a commit to Centril/rust that referenced this pull requestJul 26, 2019
…estebanklibrustc_errors: Support ui-testing flag in annotate-snippet emitterThis adds support for the `-Z ui-testing` flag to the newannotate-snippet diagnostic emitter.Support for the flag was added to `annotate-snippet-rs` in these PRs:*rust-lang/annotate-snippets-rs#3*rust-lang/annotate-snippets-rs#5r?@estebankClosesrust-lang#61811
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@zbranieckizbranieckizbraniecki left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Provide a way to 'anonymize' line numbers when building a Snippet

3 participants

@phansch@oli-obk@zbraniecki

[8]ページ先頭

©2009-2025 Movatter.jp