Skip to content

Remove line directives from generated files. #7

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

Merged
merged 2 commits into from
Sep 12, 2018

Conversation

allevato
Copy link
Member

@allevato allevato commented Sep 7, 2018

These just add noise in the generated files and contain absolute file
paths on whosever filesystem the files were generated, so they aren't
terribly useful.

These just add noise in the generated files and contain absolute file
paths on whosever filesystem the files were generated, so they aren't
terribly useful.
@allevato allevato requested a review from ahoppen September 7, 2018 08:14
@jrose-apple
Copy link

They're not so useful for the snapshots, but they would be useful when working locally, no?

@allevato
Copy link
Member Author

allevato commented Sep 7, 2018

I'll defer to those with far more GYB experience than I have 🙂

I'm happy to add a new command line flag that preserves them for local development but leaves them out by default for snapshots, or vice versa.

@dabrahams
Copy link
Contributor

dabrahams commented Sep 7, 2018 via email

@ahoppen
Copy link
Member

ahoppen commented Sep 7, 2018

I agree that the experience of looking at the gyb-soucre is not terribly enlightening because the generation logic is oft too complex to grasp instantly while the generated source is. Still, taking the option away entirely seems like a regression.

I would suggest not generating the line directives by default (which will also stop them from showing up in the generated snapshots) but adding an option like --add-line-directives to build-script.py which enables the line directive generation again. What do you think?

@allevato
Copy link
Member Author

allevato commented Sep 7, 2018

An option sounds good to me. I've named it --add-source-locations, just to make it align better with the ###sourceLocation(...) text that's inserted into the source.

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

LGTM now

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

LGTM

@rintaro
Copy link
Member

rintaro commented Sep 12, 2018

@shahmishal
Copy link
Member

@swift-ci test

@swiftlang swiftlang deleted a comment from swift-ci Sep 12, 2018
@swiftlang swiftlang deleted a comment from swift-ci Sep 12, 2018
Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

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

LGTM!

@nkcsgexi nkcsgexi merged commit 7eca798 into swiftlang:master Sep 12, 2018
@allevato allevato deleted the remove-line-directives branch September 17, 2018 18:46
allevato pushed a commit to allevato/swift-syntax that referenced this pull request Sep 17, 2018
Remove line directives from generated files.
nkcsgexi added a commit that referenced this pull request Sep 17, 2018
[4.2] Merge pull request #7 from allevato/remove-line-directives
CippoX added a commit to CippoX/swift-syntax that referenced this pull request Mar 21, 2023
# This is the 1st commit message:

fixed testAvailabilityQuery34 and testAvailabilityQueryUnavailability28

# This is the commit message swiftlang#2:

Update Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Co-authored-by: Kim de Vos <[email protected]>
# This is the commit message swiftlang#3:

added fixedSource in test case

# This is the commit message swiftlang#4:

minor changes

# This is the commit message swiftlang#5:

implemented recovery inside the parser

# This is the commit message swiftlang#6:

runned format.py

# This is the commit message swiftlang#7:

minor changes

# This is the commit message swiftlang#8:

minor changes
adevress pushed a commit to adevress/swift-syntax that referenced this pull request Jan 14, 2024
[Gardening] Rename fill(_:into:) to fill(parseResult:into:)
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.

7 participants