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

Avoid parsing snippets with placeholders #5858

Merged
merged 1 commit into from
Feb 3, 2022

Conversation

anthony-c-martin
Copy link
Member

@anthony-c-martin anthony-c-martin commented Feb 3, 2022

On my other branch, I was running into a huge number of ExpectedTokenException errors on startup, because the SnippetProvider processes the snippets after placeholder replacement - at which point in time, the file is not a syntactically valid Bicep file. This is unnecessary, and in my branch actually resulted in invalid snippets being generated due to syntax ambiguity.

Here I've modified SnippetProvider to do its analysis on the snippets with placeholders instead (which are validated for parse errors on checkin).

To give a concrete example, previously we were trying to parse the following (note that ${ and } are not recognized as valid Bicep syntax:

resource ${1:aksCluster} 'Microsoft.ContainerService/managedClusters@2021-03-01' = {
  name: ${2:'name'}
  location: ${3:location}
...

Now we're trying to parse the following:

resource /*${1:aksCluster}*/aksCluster 'Microsoft.ContainerService/managedClusters@2021-03-01' = {
  name: /*${2:'name'}*/'name'
  location: /*${3:location}*/'location'
...

@anthony-c-martin anthony-c-martin enabled auto-merge (squash) February 3, 2022 16:41
// To avoid recomputing spans, we will perform the replacements in reverse order
foreach (var match in matches.OrderByDescending(x => x.Index))
{
buffer.Replace(oldValue: match.Value,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatting is off.

@@ -147,11 +147,11 @@ public enum BicepCompletionContextKind
/// <summary>
/// The current location is after # sign.
/// </summary>
DisableNextLineDiagnosticsDirectiveStart = 1 << 25,
DisableNextLineDiagnosticsDirectiveStart = 1 << 26,
Copy link
Contributor

@bhsubra bhsubra Feb 3, 2022

Choose a reason for hiding this comment

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

Are these applicable to changes in this PR?

@anthony-c-martin anthony-c-martin merged commit ba6ef34 into main Feb 3, 2022
@anthony-c-martin anthony-c-martin deleted the ant/snippets_no_parse_errors branch February 3, 2022 17:48
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