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

Add suggestion for incorrect field syntax. #57863

Merged
merged 1 commit into from
Jan 25, 2019
Merged

Add suggestion for incorrect field syntax. #57863

merged 1 commit into from
Jan 25, 2019

Conversation

davidtwco
Copy link
Member

Fixes #57684.

This commit adds a suggestion when a = character is used when
specifying the value of a field in a struct constructor incorrectly
instead of a : character.

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 23, 2019
@davidtwco
Copy link
Member Author

Sorry for another addition to your review queue @estebank. Not sure if this is the approach we want to take with this issue or if it is too large a change for the potential gain - if we just want to change to keep the existing error but add : (as below) then I can revisit this and do that:

error: expected one of `:`, `,` or `}`, found `=`

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 23, 2019

📌 Commit 517840b3124c859bdfcd5e3d0ec8666d036dcb9f has been approved by estebank

@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 Jan 23, 2019
@estebank
Copy link
Contributor

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 23, 2019
@davidtwco
Copy link
Member Author

Since only ':' is expected, this should just state "expected :, found =".

I leant toward matching the structure of the existing messages - expected one of X, found Y instead of omitting the "one of". I can change this if we prefer otherwise.

@ljedrz
Copy link
Contributor

ljedrz commented Jan 23, 2019

I just feel this might sound like a piece of the error is missing, but it's not a big deal, especially if this is an existing convention for singular items (in which case it might be a better idea to potentially adjust the convention).

@estebank
Copy link
Contributor

Yeah, sorry about that it was my typo, my bad. It should be "expected :, found =", I had originally written "expected one of :, , or }, found =" and chose against that to make it even clearer for this case in particular. Can you change it to that?

@davidtwco
Add suggestion for incorrect field syntax.
f14d007 
This commit adds a suggestion when a `=` character is used when
specifying the value of a field in a struct constructor incorrectly
instead of a `:` character.
@davidtwco
Copy link
Member Author

@estebank should be fixed now.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 23, 2019

📌 Commit f14d007 has been approved by estebank

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 23, 2019
Centril added a commit to Centril/rust that referenced this pull request Jan 24, 2019
@Centril
Rollup merge of rust-lang#57863 - davidtwco:issue-57684, r=estebank
8470dad 
Add suggestion for incorrect field syntax.

Fixes rust-lang#57684.

This commit adds a suggestion when a `=` character is used when
specifying the value of a field in a struct constructor incorrectly
instead of a `:` character.

r? @estebank
@Centril Centril mentioned this pull request Jan 24, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jan 24, 2019
@Centril
Rollup merge of rust-lang#57863 - davidtwco:issue-57684, r=estebank
1a3b3d4 
Add suggestion for incorrect field syntax.

Fixes rust-lang#57684.

This commit adds a suggestion when a `=` character is used when
specifying the value of a field in a struct constructor incorrectly
instead of a `:` character.

r? @estebank
@Centril Centril mentioned this pull request Jan 24, 2019
Merged
bors added a commit that referenced this pull request Jan 24, 2019
@bors
Auto merge of #57879 - Centril:rollup, r=Centril
278067d 
Rollup of 9 pull requests

Successful merges:

 - #57380 (Fix Instant/Duration math precision & associativity on Windows)
 - #57606 (Get rid of the fake stack frame for reading from constants)
 - #57803 (Several changes to libunwind for SGX target)
 - #57846 (rustdoc: fix ICE from loading proc-macro stubs)
 - #57860 (Add os::fortanix_sgx::ffi module)
 - #57861 (Don't export table by default in wasm)
 - #57863 (Add suggestion for incorrect field syntax.)
 - #57867 (Fix std::future::from_generator documentation)
 - #57873 (Stabilize no_panic_pow)

Failed merges:

r? @ghost
@bors bors merged commit f14d007 into rust-lang:master Jan 25, 2019
@davidtwco davidtwco deleted the issue-57684 branch January 25, 2019 10:09
@pnkfelix pnkfelix mentioned this pull request Mar 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

@ljedrz ljedrz ljedrz left review comments

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@estebank estebank

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@davidtwco @estebank @bors @ljedrz @petrochenkov @rust-highfive