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

Tracking issue for RFC 1682: field init shorthand #37340

Closed
aturon opened this Issue Oct 22, 2016 · 30 comments

Comments

Projects
None yet
@aturon
Copy link
Member

aturon commented Oct 22, 2016

RFC.

Before stabilizing:

  • Needs documentation (#38830)
@Havvy

This comment has been minimized.

Copy link
Contributor

Havvy commented Oct 22, 2016

Once we have this syntax, we'll want to clean up the compiler where it's useful. Since this would be almost a rote cleanup, we'd either want a mentored issue or a tool to do it.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Oct 22, 2016

a tool to do it.

I wonder if @killercup 's rustfix could do this

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Oct 22, 2016

@steveklabnik rustfmt doing it by default has also been mentioned. cc @nrc

@nrc

This comment has been minimized.

Copy link
Member

nrc commented Oct 24, 2016

Yeah, Rustfmt could def. do this.

@killercup

This comment has been minimized.

Copy link
Member

killercup commented Oct 25, 2016

I haven't read that RFC; if you can write a lint that suggests the new
syntax (with a concrete and valid code suggestion), rustfix will be able to
fix this. (If this is only a syntactic change though it might indeed make
more sense to add this to rustfmt.)

Eduard-Mihai Burtescu notifications@github.com schrieb am Sa. 22. Okt.
2016 um 09:54:

@steveklabnik https://github.com/steveklabnik rustfmt doing it by
default has also been mentioned. cc @nrc https://github.com/nrc


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

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Oct 25, 2016

I think that the compiler shouldn't lint for this, suggesting sugar features is more a thing for clippy.

bors added a commit that referenced this issue Oct 27, 2016

Auto merge of #11994 - eddyb:struct-literal-field-shorthand, r=nrc
Implement field shorthands in struct literal expressions.

Implements #37340 in a straight-forward way: `Foo { x, y: f() }` parses as `Foo { x: x, y: f() }`.
Because of the added `is_shorthand` to `ast::Field`, this is `[syntax-breaking]` (cc @Manishearth).

* [x] Mark the fields as being a shorthand (the exact same way we do it in patterns), for pretty-printing.
* [x] Gate the shorthand syntax with `#![feature(field_init_shorthand)]`.
* [x] Don't parse numeric field as identifiers.
* [x] Arbitrary field order tests.
@chriskrycho

This comment has been minimized.

Copy link
Contributor

chriskrycho commented Dec 23, 2016

What's the current status on this—in nightly, it looks like, via #11994? Do we know when it's expected to land?

@matthew-piziak

This comment has been minimized.

Copy link
Contributor

matthew-piziak commented Dec 23, 2016

@chriskrycho Confirmed—it works in nightly. I'm using the feature in my nightly project right now.

@Havvy

This comment has been minimized.

Copy link
Contributor

Havvy commented Dec 25, 2016

The new documentation required RFC means we need to figure out how to document it, I think?

  • The grammar needs updating
  • as would adding a section to the book and the new book.
  • The reference would need a paragraph explaining the sugar.
  • Additionally, we may want to add examples to Rust By Example showing the syntax.

We also need changelog entries? I propose the following for the short entry. I don't have the energy right now to try a long entry.

Added field init shorthand syntax when the field and variable names match.

@matthew-piziak

This comment has been minimized.

Copy link
Contributor

matthew-piziak commented Dec 25, 2016

Rustfmt also needs support for this feature. The tracking issue is here. I'm planning to tackle that eventually but I'm not going to lick the cookie just yet.

@nikomatsakis nikomatsakis referenced this issue Jan 4, 2017

Closed

document field init shorthand #38830

4 of 5 tasks complete
@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 4, 2017

I opened #38830 to specifically track adding docs.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jan 11, 2017

Can we nominate this for FCP now in expectation that it will be finished in time to branch 1.16 on Feb 2?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 11, 2017

@rfcbot fcp merge

I propose that we stabilize field init shorthand. It's handy and uncomplicated. One remaining work item is documentation, which is being tracked by #38830, but work has already begun.

@nikomatsakis nikomatsakis added the T-lang label Jan 11, 2017

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Jan 11, 2017

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@nrc

This comment has been minimized.

Copy link
Member

nrc commented Jan 11, 2017

Reluctantly, and assuming documentation is finished before we stabilise.

@withoutboats

This comment has been minimized.

Copy link
Contributor

withoutboats commented Jan 11, 2017

@rfcbot reviewed

@nrc RFC 1636 requires that we document every feature before we actually stabilize it.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 12, 2017

@rfcbot concern documentation-unfinished

(Despite the point just made by @withoutboats, I see no reason to put my check mark on something whose required documentation is unfinished.)

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Jan 12, 2017

@pnkfelix

I see no reason to put my check mark on something whose required documentation is unfinished.)

There has been some discussion about this question in the thread to stabilize custom derive.

quoting @steveklabnik :

My imagining of this was always that the decision to stabilize and the actual stabilization are separate. That is, @rust-lang/lang can say "this can be made stable", but the commit to remove the gate also ensures the documentation exists.

Given that we just had a release, my plan was to do something like this:

  1. Wait for this to leave FCP
  2. Land some docs. (I was planning on writing them in this case)
  3. Make the stabilization PR.

Possibly combining two and three.

In my opinion, it would make sense to separate the two decisions as it allows for a cleaner transition. E.g. you can't merge a docs PR if you don't know whether something will be stabilized (docs are intended for the users, no? They should, in the best case, always match what is actually stable), but if there is no FCP, how do you know whether it will actually be stabilized.

@aturon

This comment has been minimized.

Copy link
Member Author

aturon commented Jan 12, 2017

@est31 I entirely agree. The review here is purely to go into FCP, which still leaves several weeks before stabilization could even occur; the check for documentation should happen separately, later. That will give us the highest throughput.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 12, 2017

I can see the throughput argument.

But I am having difficulty making this argument jibe in my head with the fact that RFC's now have a section on "how this will be taught to new users" ... so I thought we were transitioning to a world where docs were required earlier on before we make final decisions on them.

Anyway, fine, I don't actually care enough to hold up the process.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 12, 2017

@rfcbot resolved documentation-unfinished

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 12, 2017

@rfcbot reviewed

@aturon

This comment has been minimized.

Copy link
Member Author

aturon commented Jan 13, 2017

Manually marking for FCP.

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Jan 23, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @nikomatsakis, I wasn't able to add the final-comment-period label, please do so.

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Feb 2, 2017

The final comment period is now complete.

@lfairy

This comment has been minimized.

Copy link
Contributor

lfairy commented Feb 12, 2017

Now that #39459 has landed, I think there's nothing blocking stabilization at this point.

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Feb 12, 2017

@lfairy I'm not sure, #38830 is still open, no?

est31 added a commit to est31/rust that referenced this issue Feb 12, 2017

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Feb 12, 2017

Hmm... seems @lfairy is right: the documentation RFC doesn't require that the feature is documented in Rust by example. So all the points required for stabilisation are done (see the list in #38830), which means we can stabilize it!

est31 added a commit to est31/rust that referenced this issue Feb 15, 2017

@aturon

This comment has been minimized.

Copy link
Member Author

aturon commented Feb 15, 2017

We're ready to go forward with stabilization!

@chriskrycho

This comment has been minimized.

Copy link
Contributor

chriskrycho commented Feb 16, 2017

Easy enough to add an example anyway. I needed a little win today!

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 16, 2017

Rollup merge of rust-lang#39761 - est31:master, r=aturon
Stabilize field init shorthand

Closes rust-lang#37340.

~Still blocked by the documentation issue rust-lang#38830.~ EDIT: seems that all parts required for stabilisation are fixed, so its not blocked.

bors added a commit that referenced this issue Feb 16, 2017

Auto merge of #39761 - est31:master, r=aturon
Stabilize field init shorthand

Closes #37340.

~Still blocked by the documentation issue #38830.~ EDIT: seems that all parts required for stabilisation are fixed, so its not blocked.

@bors bors closed this in #39761 Feb 16, 2017

bors added a commit that referenced this issue Jul 5, 2017

Auto merge of #43008 - zackmdavis:field_init_shorthand_in_librustc, r…
…=<try>

use field init shorthand in src/librustc/

Commentary on #37340 [suggested](#37340 (comment)) using the new field init syntax in the compiler. Do we care about this? If so, here's a pull request for the librustc/ directory. While [`rustfmt` might do this in the future](#37340 (comment)), in the meantime, some simple Python will do:

```python
#!/usr/bin/env python3

import os, re, sys

OPPORTUNITY = re.compile(r" (\w+): \1,?\n")

def field_init_shorthand_substitution(filename):
    with open(filename) as f:
        text = f.read()
        revised = OPPORTUNITY.sub(r" \1,\n", text)
    with open(filename, 'w') as f:
        f.write(revised)

def substitute_in_directory(path):
    for dirname, _subdirs, basenames in os.walk(path):
        for basename in basenames:
            field_init_shorthand_substitution(os.path.join(dirname, basename))

if __name__ == "__main__":
    substitute_in_directory(sys.argv[1])
```

**Update 3 July**: edited the search (respectively replace) regex to ` (\w+): \1,?\n` (` \1,\n`) from ` (\w+): \1,` (` \1,`)

bors added a commit that referenced this issue Jul 6, 2017

Auto merge of #43008 - zackmdavis:field_init_shorthand_in_librustc, r…
…=estebank

use field init shorthand in src/librustc/

Commentary on #37340 [suggested](#37340 (comment)) using the new field init syntax in the compiler. Do we care about this? If so, here's a pull request for the librustc/ directory. While [`rustfmt` might do this in the future](#37340 (comment)), in the meantime, some simple Python will do:

```python
#!/usr/bin/env python3

import os, re, sys

OPPORTUNITY = re.compile(r" (\w+): \1,?\n")

def field_init_shorthand_substitution(filename):
    with open(filename) as f:
        text = f.read()
        revised = OPPORTUNITY.sub(r" \1,\n", text)
    with open(filename, 'w') as f:
        f.write(revised)

def substitute_in_directory(path):
    for dirname, _subdirs, basenames in os.walk(path):
        for basename in basenames:
            field_init_shorthand_substitution(os.path.join(dirname, basename))

if __name__ == "__main__":
    substitute_in_directory(sys.argv[1])
```

**Update 3 July**: edited the search (respectively replace) regex to ` (\w+): \1,?\n` (` \1,\n`) from ` (\w+): \1,` (` \1,`)

@ghost ghost referenced this issue Jul 17, 2017

Closed

cargo install fails #47

dlrobertson pushed a commit to dlrobertson/rust that referenced this issue Nov 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.