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

Make all vec! macros use square brackets: Attempt 2 #37497

Merged
merged 1 commit into from
Nov 1, 2016

Conversation

iirelu
Copy link
Contributor

@iirelu iirelu commented Oct 31, 2016

The last PR ended with tears after a valiant struggle with git. I managed to clean up the completely broken history of that into a brand spanking new PR! Yay!

Original:

Everyone hates the old syntax. I hope. Otherwise this PR has some controversy I wasn't expecting.

This would be the perfect time to write a lint recommending vec![..] when you use another style.

Disclaimer: I may have broken something. If I have, I'll fix them when the tests come in. Luckily the chance for a non-syntactical error is pretty low in all this.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (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.

@sanxiyn
Copy link
Member

sanxiyn commented Oct 31, 2016

This still has merge commits...

@retep998
Copy link
Member

retep998 commented Oct 31, 2016

There's a little checkbox you can check to allow the maintainers of this project to be able to update your PR for you, which would let them clean up your commit history for you. https://github.com/blog/2247-improving-collaboration-with-forks

@iirelu
Copy link
Contributor Author

iirelu commented Oct 31, 2016

Okay, I think I finally unfricked my commit history-- AW GOD DAMMIT
b

Most of the Rust community agrees that the vec! macro is clearer when
called using square brackets [] instead of regular brackets (). Most of
these ocurrences are from before macros allowed using different types of
brackets.

There is one left unchanged in a pretty-print test, as the pretty
printer still wants it to have regular brackets.
@iirelu
Copy link
Contributor Author

iirelu commented Oct 31, 2016

After countless horrible force-pushes, history rewrites, and endless trial and error, I have finally tamed the beast known as git. Thanks so much to @steveklabnik for all the help! 💜

@steveklabnik
Copy link
Member

@bors: r+ p=1

Let's try to get this in the queue so it doesn't go out of date.

@bors
Copy link
Contributor

bors commented Oct 31, 2016

📌 Commit e593c3b has been approved by steveklabnik

@steveklabnik
Copy link
Member

(That is, I'll r- if Travis still fails for some reason.)

@bors
Copy link
Contributor

bors commented Oct 31, 2016

⌛ Testing commit e593c3b with merge 69ec350...

bors added a commit that referenced this pull request Oct 31, 2016
Make all vec! macros use square brackets: Attempt 2

[The last PR](#37476) ended with tears after a valiant struggle with git. I managed to clean up the completely broken history of that into a brand spanking new PR! Yay!

Original:

> Everyone hates the old syntax. I hope. Otherwise this PR has some controversy I wasn't expecting.

> This would be the perfect time to write a lint recommending vec![..] when you use another style.

> Disclaimer: I may have broken something. If I have, I'll fix them when the tests come in. Luckily the chance for a non-syntactical error is pretty low in all this.
@steveklabnik
Copy link
Member

Well looks like @bors got to it before Travis, I'm sure it'll be fine 😄

@bors bors merged commit e593c3b into rust-lang:master Nov 1, 2016
@WiSaGaN
Copy link
Contributor

WiSaGaN commented Nov 1, 2016

Has there been a discussion why we prefer vec![] to vec!()?

@retep998
Copy link
Member

retep998 commented Nov 1, 2016

@WiSaGaN Because it matches array literal syntax which uses [].

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Nov 1, 2016

Seems it can go either way as vec!() is consistent with how other macros get called while vec![] is not.

@steveklabnik
Copy link
Member

Not all macros get called with (). The usual idiom is, macros that look
like function calls use (), macros that deal with things like vectors or
arrays use [], and macros that involve wrapping code are invoked with
{}. All syntaxes are supported for any macro, but that tends to be idiom.

On Mon, Oct 31, 2016 at 11:38 PM, Wangshan Lu notifications@github.com
wrote:

Seems it can go either way as vec!() is consistent with how other macros
get called while vec![] is not.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#37497 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABsioJgFzhjuTtJ_nKRZUjoFf-y8LQVks5q5rQxgaJpZM4Kk_Yt
.

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.

8 participants