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

Loop snippets #1647

Merged
merged 2 commits into from
Mar 1, 2021
Merged

Loop snippets #1647

merged 2 commits into from
Mar 1, 2021

Conversation

majastrz
Copy link
Member

Added loop snippets to simplify loop authoring. The completions that insert the snippets are always available on module/resource values. On property value completions, we only include the loop snippet completion if the property is of array type and the new property loop won't be nested under an existing one.

This fixes #1616.

LoopSnippets2

@@ -90,8 +111,9 @@ public override void VisitForSyntax(ForSyntax syntax)
case null:
// this is a property loop
this.propertyLoopCount += 1;
this.maximumPropertyLoopCount = Math.Max(this.maximumPropertyLoopCount, this.propertyLoopCount);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of maximumPropertyLoopCount? It is assigned but its value doesn't seem to be used anywhere.

Copy link
Member Author

@majastrz majastrz Feb 27, 2021

Choose a reason for hiding this comment

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

Was originally trying to implement this by partially running the same visitor logic. This was needed for that but I forgot to remove it when I found a simple way.

I'll remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed it

@majastrz majastrz enabled auto-merge (squash) February 28, 2021 11:15
@codecov-io
Copy link

codecov-io commented Feb 28, 2021

Codecov Report

Merging #1647 (1d73728) into main (9a5be60) will decrease coverage by 0.00%.
The diff coverage is 96.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1647      +/-   ##
==========================================
- Coverage   95.15%   95.15%   -0.01%     
==========================================
  Files         363      363              
  Lines       20098    20138      +40     
  Branches       13       13              
==========================================
+ Hits        19125    19162      +37     
- Misses        973      976       +3     
Flag Coverage Δ
dotnet 95.61% <96.55%> (-0.01%) ⬇️
typescript 27.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
....LangServer/Completions/BicepCompletionProvider.cs 96.54% <94.73%> (-0.14%) ⬇️
src/Bicep.Core/Emit/ForSyntaxValidatorVisitor.cs 100.00% <100.00%> (ø)
src/Bicep.Core/Emit/ResourceDependencyVisitor.cs 93.33% <0.00%> (-1.75%) ⬇️
src/Bicep.Core/Syntax/ResourceDeclarationSyntax.cs 97.87% <0.00%> (+0.04%) ⬆️
src/Bicep.Core/Syntax/ModuleDeclarationSyntax.cs 97.67% <0.00%> (+0.05%) ⬆️
src/Bicep.Core/DataFlow/DataFlowAnalyzer.cs 92.85% <0.00%> (+0.54%) ⬆️

@majastrz majastrz merged commit ec7a4b0 into main Mar 1, 2021
@majastrz majastrz deleted the majastrz/loopy-snippet branch March 1, 2021 14:19
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.

Implement loop snippets
4 participants