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

Better handling of tab in error #33513

Merged
merged 1 commit into from
May 13, 2016
Merged

Better handling of tab in error #33513

merged 1 commit into from
May 13, 2016

Conversation

sanxiyn
Copy link
Member

@sanxiyn sanxiyn commented May 9, 2016

cc #33240.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@sanxiyn
Copy link
Member Author

sanxiyn commented May 9, 2016

Ugh, this failed tidy because of (intentional) tab characters.

@sanxiyn sanxiyn mentioned this pull request May 11, 2016
Closed
10 tasks
@nikomatsakis
Copy link
Contributor

thanks @sanxiyn! I'll take a look soon.

@nikomatsakis
Copy link
Contributor

@sanxiyn

Ugh, this failed tidy because of (intentional) tab characters.

I think you can just change the string constant to use a multi-line string "" string with \t, that'd probably be better anyway (more obvious that a tab is present).

I took a quick look at the PR. I'll look a bit more deeply in a bit. This seems like an improvement over the status quo, though I had contemplated some more radical approaches -- for example modifying the source text to convert tabs to spaces, and adjusting column numbers in the annotations appropriately, or maintaining some more complex mapping (the aim there would be to also handle unicode characters more gracefully, I think we have some challenges there still based on the unicode-input tests).

@sanxiyn
Copy link
Member Author

sanxiyn commented May 12, 2016

Changed to use \t.

We could do something more fancy later. This is simply restoring the longstanding behavior introduced in fd98ea8, thereby fixing the regression.

@nikomatsakis
Copy link
Contributor

Yeah, good for now. Thanks @sanxiyn ! :)

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 12, 2016

📌 Commit c331032 has been approved by nikomatsakis

@eddyb eddyb mentioned this pull request May 12, 2016
Closed
eddyb added a commit to eddyb/rust that referenced this pull request May 12, 2016
@eddyb
Rollup merge of rust-lang#33513 - sanxiyn:tab-in-error, r=nikomatsakis
be59d3f 
Better handling of tab in error

cc rust-lang#33240.
@eddyb eddyb mentioned this pull request May 12, 2016
Closed
bors added a commit that referenced this pull request May 12, 2016
@bors
Auto merge of #33597 - eddyb:rollup, r=eddyb
2c5ce8c 
Rollup of 23 pull requests

- Successful merges: #33282, #33342, #33393, #33450, #33513, #33517, #33531, #33532, #33533, #33534, #33538, #33541, #33544, #33552, #33555, #33560, #33563, #33565, #33566, #33572, #33580, #33590, #33596
- Failed merges: #33578
@eddyb eddyb mentioned this pull request May 13, 2016
Closed
eddyb added a commit to eddyb/rust that referenced this pull request May 13, 2016
@eddyb
Rollup merge of rust-lang#33513 - sanxiyn:tab-in-error, r=nikomatsakis
82fd37c 
Better handling of tab in error

cc rust-lang#33240.
@bors
Copy link
Contributor

bors commented May 13, 2016

⌛ Testing commit c331032 with merge edb6f83...

bors added a commit that referenced this pull request May 13, 2016
@bors
Auto merge of #33513 - sanxiyn:tab-in-error, r=nikomatsakis
edb6f83 
Better handling of tab in error

cc #33240.
@bors
Copy link
Contributor

bors commented May 13, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, auto-linux-64-cargotest, auto-linux-64-cross-armhf, auto-linux-64-cross-armsf, auto-linux-64-cross-freebsd, auto-linux-64-cross-netbsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild

@bors bors merged commit c331032 into rust-lang:master May 13, 2016
@sanxiyn sanxiyn deleted the tab-in-error branch May 14, 2016 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sanxiyn @rust-highfive @nikomatsakis @bors