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

Fix std::fmt format spec: named args are allowed with "$" syntax #33277

Merged
merged 1 commit into from
May 5, 2016
Merged

Fix std::fmt format spec: named args are allowed with "$" syntax #33277

merged 1 commit into from
May 5, 2016

Conversation

birkenfeld
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Collaborator

r? @aturon

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

GuillaumeGomez
GuillaumeGomez reviewed Apr 29, 2016
View reviewed changes
src/libcollections/fmt.rs
@@ -456,6 +456,10 @@
//! // Hello {next arg (x)} is {arg 2 (0.01) with precision
//! // specified in its predecessor (5)}
//! println!("Hello {} is {2:.*}", "x", 5, 0.01);
//!
//! // Hello {next arg (x)} is {arg "number" (0.01) with precision specified
//! in arg "prec" (5)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing "//" at the beginning of the line.

@GuillaumeGomez
Copy link
Member

Except for the nit, everything's good. Thanks for the PR. Ping me once it's fixed.

@birkenfeld
Copy link
Contributor Author

@GuillaumeGomez thanks, fixed.

steveklabnik
steveklabnik reviewed Apr 29, 2016
View reviewed changes
src/libcollections/fmt.rs
@@ -333,7 +333,7 @@
//! precision := count | '*'
//! type := identifier | ''
//! count := parameter | integer
//! parameter := integer '$'
//! parameter := argument '$'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't we want this to be integer or argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argument is defined above as integer | identifier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I missed that 👍

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Apr 29, 2016

📌 Commit 84bd1ce has been approved by steveklabnik

@bluss
Copy link
Member

bluss commented Apr 29, 2016

🌟 Nice to get this into the docs, then it also gets into rust code out there eventually

@bors
Copy link
Contributor

bors commented Apr 30, 2016

⌛ Testing commit 84bd1ce with merge daf281d...

bors added a commit that referenced this pull request Apr 30, 2016
@bors
Auto merge of #33277 - birkenfeld:fmt-named-dollar-args, r=steveklabnik
daf281d 
Fix std::fmt format spec: named args are allowed with "$" syntax
@bors
Copy link
Contributor

bors commented Apr 30, 2016

💔 Test failed - auto-mac-64-opt-rustbuild

@alexcrichton
Copy link
Member

@bors: retry

On Sat, Apr 30, 2016 at 12:59 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-mac-64-opt-rustbuild
http://buildbot.rust-lang.org/builders/auto-mac-64-opt-rustbuild/builds/931

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#33277 (comment)

@bors
Copy link
Contributor

bors commented Apr 30, 2016

⚡ Previous build results for auto-linux-64-opt-rustbuild, auto-win-msvc-64-cargotest are reusable. Rebuilding only 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-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-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild...

@bors
Copy link
Contributor

bors commented Apr 30, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@alexcrichton
Copy link
Member

@bors: retry

On Sat, Apr 30, 2016 at 3:02 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-win-gnu-32-opt-rustbuild
http://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt-rustbuild/builds/940

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#33277 (comment)

@bors
Copy link
Contributor

bors commented May 1, 2016

⚡ Previous build results for auto-linux-64-cargotest, auto-linux-64-opt-rustbuild, auto-win-msvc-64-cargotest are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, 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-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-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild...

@bors
Copy link
Contributor

bors commented May 1, 2016

💔 Test failed - auto-linux-64-opt-mir

@alexcrichton
Copy link
Member

@bors: retry

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 2, 2016
@steveklabnik
Rollup merge of rust-lang#33277 - birkenfeld:fmt-named-dollar-args, r…
fc5d99b 
…=steveklabnik

Fix std::fmt format spec: named args are allowed with "$" syntax
@steveklabnik steveklabnik mentioned this pull request May 2, 2016
Closed
@Manishearth Manishearth mentioned this pull request May 3, 2016
Merged
Manishearth added a commit to Manishearth/rust that referenced this pull request May 3, 2016
@Manishearth
Rollup merge of rust-lang#33277 - birkenfeld:fmt-named-dollar-args, r…
af05b56 
…=steveklabnik

Fix std::fmt format spec: named args are allowed with "$" syntax
bors added a commit that referenced this pull request May 3, 2016
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
1fb4b5b 
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
bors added a commit that referenced this pull request May 3, 2016
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
f7125fc 
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
@alexcrichton
Copy link
Member

@bors: retry

bors added a commit that referenced this pull request May 4, 2016
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
1895d08 
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
bors added a commit that referenced this pull request May 4, 2016
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
7a0ccc4 
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
@bors bors merged commit 84bd1ce into rust-lang:master May 5, 2016
@birkenfeld birkenfeld deleted the fmt-named-dollar-args branch May 6, 2016 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@aturon aturon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@birkenfeld @rust-highfive @GuillaumeGomez @steveklabnik @bors @bluss @alexcrichton @aturon