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

Keep as much span info as possible #252

Merged
merged 3 commits into from Sep 17, 2019
Merged

Keep as much span info as possible #252

merged 3 commits into from Sep 17, 2019

Conversation

CreepySkeleton
Copy link
Collaborator

@CreepySkeleton CreepySkeleton commented Sep 7, 2019

Closes #247
Work-in-progress, just so you could state your opinion on this changes

Em, fix to #156 is also here, I can extract it to a separate PR if you want

@CreepySkeleton CreepySkeleton marked this pull request as ready for review September 7, 2019 00:06
@CreepySkeleton
Copy link
Collaborator Author

@TeXitoi Pretty much ready, waiting on review

@CreepySkeleton
Copy link
Collaborator Author

ping @TeXitoi , sorry for being annoying but I need your opinion to move on

Copy link
Owner

@TeXitoi TeXitoi left a comment

Choose a reason for hiding this comment

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

I lightly reviewed and it seems fine to me.

structopt-derive/src/attrs.rs Outdated Show resolved Hide resolved
@TeXitoi
Copy link
Owner

TeXitoi commented Sep 9, 2019

for #156 a separate commit would be great.

@CreepySkeleton CreepySkeleton changed the title Spans Keep span info as much as possible Sep 9, 2019
@CreepySkeleton
Copy link
Collaborator Author

Well, that's final line. I'll go work on AST implementation, should be done in a week or so (I'm kind of overloaded by work/life stuff right now, sorry)

@CreepySkeleton
Copy link
Collaborator Author

ping @TeXitoi

@CreepySkeleton
Copy link
Collaborator Author

cc @TeXitoi

Copy link
Owner

@TeXitoi TeXitoi left a comment

Choose a reason for hiding this comment

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

Changelog entry and I merge.

Sorry to be long to review.

@CreepySkeleton
Copy link
Collaborator Author

Changelog entry and I merge.

@TeXitoi
I don't think there's much to put in changelog about #247. These changes are purely cosmetical

@TeXitoi
Copy link
Owner

TeXitoi commented Sep 17, 2019

There is an error message change that fail the nightly build :-(

@CreepySkeleton
Copy link
Collaborator Author

@TeXitoi They look identical. I'll investigate in few hours

@TeXitoi
Copy link
Owner

TeXitoi commented Sep 17, 2019

There is an extra message after the ^^^^^^^^

@CreepySkeleton CreepySkeleton changed the title Keep span info as much as possible Keep as much span info as possible Sep 17, 2019
@CreepySkeleton
Copy link
Collaborator Author

Fucking nightly! Will fix

Most of `quote!` invocations are replaced with `quote_spanned!` ones.
Now everywhere - sometimes it's pointless, sometimes we don't have
any meaningless location to toke a span from, sometimes I just can't
workaround the current implementation - too much changes.
@TeXitoi TeXitoi merged commit fe9e049 into TeXitoi:master Sep 17, 2019
@TeXitoi
Copy link
Owner

TeXitoi commented Sep 17, 2019

Thanks.

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.

Most of the generated code should carry span info about its origins
2 participants