Skip to content

Add diagnostic message for identifier editor placeholder #1380

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

Conversation

kimdv
Copy link
Contributor

@kimdv kimdv commented Mar 2, 2023

Part of fixing an TODOs in here: #1373

I just wanna ask if I'm on the right path.

@kimdv kimdv marked this pull request as ready for review March 3, 2023 08:27
@kimdv kimdv requested a review from ahoppen as a code owner March 3, 2023 08:27
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.

This won’t work as-is because there are other places that also consume an identifier and would consume the editor placeholder as a normal identifier without emitting an error.

Instead, I think the error should be emitted by the Lever. You can take a look at Cursor.swift. There are a few cases that add a LexerDiagnostic. We should do the same for editor placeholders.

@kimdv kimdv force-pushed the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch from e29a2f6 to c892162 Compare March 9, 2023 09:41
@kimdv kimdv requested a review from ahoppen March 9, 2023 09:44
@kimdv
Copy link
Contributor Author

kimdv commented Mar 9, 2023

@swift-ci please test

@kimdv kimdv force-pushed the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch from c892162 to 21a3c0e Compare March 9, 2023 12:26
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.

That’s exactly what I was thinking of 🤩

@ahoppen
Copy link
Member

ahoppen commented Mar 9, 2023

@swift-ci Please test

@kimdv kimdv force-pushed the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch 2 times, most recently from 6da1002 to ef5965a Compare March 10, 2023 09:10
@kimdv
Copy link
Contributor Author

kimdv commented Mar 10, 2023

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented Mar 10, 2023

@swift-ci please test macOS

@kimdv kimdv force-pushed the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch from ef5965a to 6e50efb Compare March 16, 2023 18:25
@kimdv
Copy link
Contributor Author

kimdv commented Mar 16, 2023

swiftlang/swift#64382

@swift-ci please test

3 similar comments
@kimdv
Copy link
Contributor Author

kimdv commented Mar 19, 2023

swiftlang/swift#64382

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented Mar 21, 2023

swiftlang/swift#64382

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented Mar 22, 2023

swiftlang/swift#64382

@swift-ci please test

@kimdv kimdv force-pushed the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch from 6e50efb to c4e8030 Compare March 24, 2023 17:33
@kimdv
Copy link
Contributor Author

kimdv commented Mar 24, 2023

swiftlang/swift#64382

@swift-ci please test

@kimdv kimdv force-pushed the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch 3 times, most recently from f17ec89 to 5833797 Compare March 24, 2023 19:17
@kimdv kimdv force-pushed the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch from 5833797 to 0054642 Compare March 24, 2023 19:20
@kimdv
Copy link
Contributor Author

kimdv commented Mar 24, 2023

swiftlang/swift#64382

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented Mar 25, 2023

swiftlang/swift#64382

@swift-ci please test

@kimdv kimdv merged commit 7e099ed into swiftlang:main Mar 25, 2023
@kimdv kimdv deleted the kimdv/add-diagnostic-message-for-identifier-editor-placeholder branch March 25, 2023 10:27
kimdv added a commit to kimdv/swift-syntax that referenced this pull request Mar 25, 2023
kimdv added a commit to kimdv/swift-syntax that referenced this pull request Mar 25, 2023
ahoppen pushed a commit to ahoppen/swift-syntax that referenced this pull request Mar 31, 2023
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.

2 participants