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

Syntax highlight fixes for quoted codes and aliases after keywords #73

Merged
merged 6 commits into from
Apr 18, 2024

Conversation

jafeltra
Copy link
Collaborator

Fixes #72

This PR fixes the issue reported in #72. With the current version of the extension, having a coding with a system that uses an alias and a code that has spaces does not highlight correctly. With this branch, the following rule no longer breaks the syntax highlighting for the rest of the file:

* valueCodeableConcept = $test#"example code"

I also made a bonus update to an issue that I saw recently. When using an Alias that starts with $ after a FSH keyword, it wasn't highlighted. If this shouldn't have been highlighted, I can revert this change. With this branch, the following keyword highlight as expected:

InstanceOf: $ServiceRequest

I checked and neither of these issues appear on FSH Online because it uses slightly different syntax highlighting.

I also ran an npm audit fix and committed those changes, and I had to update the @vscode/test-electron dependency to get the tests to run again.

@jafeltra jafeltra requested a review from cmoesel April 16, 2024 14:22
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

This probably fits into the category of "just be happy it doesn't break" -- but I noticed inconsistency in the highlighing:

  • URL system and unquoted code is all brown
  • URL system and quoted code has brown system, green #, and green code
  • Alias and unquoted code is all green
  • Alias and quoted code has green alias, brown #, and brown code

So basically, it's totally different across each of those 4 possible cominations. Did you see this? Do we care?

CleanShot 2024-04-16 at 16 08 03@2x

@jafeltra
Copy link
Collaborator Author

jafeltra commented Apr 17, 2024

Alrighty, I was tempted to say we don't care, but really I did care. I think that pattern of mismatched highlighting has actually been around for a long time, but I think I found a pretty easy way to address it. I did two things to get the highlighting to be consistent:

  • I made it so that aliases that start with $ ($abc) are highlighted brown like URLs are. I thought that made sense since they represent URLs. I also made it so that the match for those ends with a space or a # since that's where an alias ends and either a new word or a code starts. That's dca537c.
  • I made it so we don't highlight fragments that are part of URLs. Technically a # can be part of a valid URL, which is why it had previously been in our regular expression for matching URLs. However, it seemed like it would be much more common in FSH to have a URL without a fragment and with a code. I couldn't think of a way to distinguish a URL ending with a fragment and a URL system with a code. So I decided to remove the # from the regular expression for a URL, that way we can highlight the URL portion brown and highlight the code portion green. I think this could lead to some mildly confusing highlighting when creating an Alias with a url that has a fragment, but I decided I was okay with that. Let me know if you are also okay with that. That's ac66df9.

image

image

Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

Alright! These look good to me! Thanks for doing the bonus round as well!

@jafeltra jafeltra merged commit 01c52d2 into master Apr 18, 2024
14 checks passed
@jafeltra jafeltra deleted the fix-quoted-code-highlight-bug branch April 18, 2024 19:52
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.

Syntax highlighting issue
2 participants