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

Fix #138: Invalid syntax color highlighting (identity not highlighting) #140

Merged
merged 1 commit into from
Nov 20, 2017

Conversation

AlexTroshkin
Copy link
Contributor

change identity_value to identity

Copy link
Member

@kburtram kburtram left a comment

Choose a reason for hiding this comment

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

@AlexTroshkin it looks like this change is removing identity_value. SSMS does color identity_value so we should keep that value in the list when adding identity. Thanks!

@AlexTroshkin
Copy link
Contributor Author

AlexTroshkin commented Nov 18, 2017

@kburtram I returned identity_value, but I'm still wondering why the SSMS highlights this. Search in Google does not find anything about the identity_value

@kburtram
Copy link
Member

@AlexTroshkin I'm not sure why identity_value is highlighted in SSMS. There are several colorization bugs we need to clean-up, particularly regarding contextual keywords being highlighted in incorrect contexts (for example, creating a column name algorithm shouldn't highlight algorithm in that context). This work is planned for a subsequent release, but for editor we're currently working on a few other bugs (such as inconsistent rendering issues, state management when switching tabs, and keyboard navigation & shortcuts).

also, fyi, I left some comments regarding our internal vs. public repo in the other active PR that you may want to read. Basically, for the next couple months we'll be porting changes to a different repo than publishing them back here as snapshot aligned with binary releases.

@kevcunnane
Copy link
Contributor

@AlexTroshkin thanks for submitting this. Could you push an extra commit for this? I have enabled our LCA webhook which decides if you need to sign a Contributor License Agreement (CLA) for this PR or not. I'll be happy to approve it but need this step to do so.

@AlexTroshkin
Copy link
Contributor Author

@kevcunnane done :) sorry for the delay, did not immediately see your message.

@kevcunnane
Copy link
Contributor

Great - that picked it up and it's approved. Thanks!

@kevcunnane kevcunnane merged commit 01d3fe6 into microsoft:master Nov 20, 2017
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.

3 participants