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

Tsunkaraneni/module file completion #1236

Merged
merged 37 commits into from
Jan 28, 2021
Merged

Conversation

TSunny007
Copy link
Contributor

@TSunny007 TSunny007 commented Dec 28, 2020

Allows for module file/path completion. User experience is expected be similar to Typescript's import * as s from '<autocompletion>'.

  • Shows icons for files or folders
  • Allows for relative path autocompletion: Doesn't provide help on absolute path, since its not natively supported by bicep.
  • Smart adjustment of replacement range as autocompletions of files should place the cursor at the end of the line, whereas a folder should keep the cursor within the path string
  • autocomplete triggers when defining a module (module m |), typing a / (affects all bicep completions), or pressing ctrl + space anywhere in the module path string.

@TSunny007
Copy link
Contributor Author

TSunny007 commented Dec 28, 2020

Putting this in Draft as I cannot leverage completionTest - currently './<autocompletion results>' are not the same as '<autocompletion results>' because while the referenced results are the same, they differ when looked at like a string ('./main.bicep' != 'main.bicep')
Solution: Allow only canonical paths only (with optional ./ for current or child directory items)

@TSunny007 TSunny007 requested a review from majastrz December 28, 2020 19:06
@codecov-io
Copy link

codecov-io commented Dec 28, 2020

Codecov Report

Merging #1236 (1ae4183) into main (4f31b12) will decrease coverage by 0.02%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1236      +/-   ##
==========================================
- Coverage   94.82%   94.80%   -0.03%     
==========================================
  Files         343      343              
  Lines       17435    17547     +112     
  Branches       14       14              
==========================================
+ Hits        16533    16635     +102     
- Misses        902      912      +10     
Flag Coverage Δ
dotnet 95.33% <91.66%> (-0.03%) ⬇️
typescript 27.20% <ø> (ø)

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

Impacted Files Coverage Δ
src/Bicep.Core/FileSystem/InMemoryFileResolver.cs 70.58% <0.00%> (-21.72%) ⬇️
src/Bicep.Core/FileSystem/PathHelper.cs 100.00% <ø> (ø)
...icep.LangServer/Handlers/BicepCompletionHandler.cs 80.00% <62.50%> (-9.48%) ⬇️
src/Bicep.Core/FileSystem/FileResolver.cs 75.00% <77.77%> (+1.66%) ⬆️
...cep.Core.UnitTests/FileSystem/FileResolverTests.cs 97.05% <94.87%> (-2.95%) ⬇️
....LangServer/Completions/BicepCompletionProvider.cs 97.45% <98.07%> (+0.08%) ⬆️
src/Bicep.Core.UnitTests/Utils/OutputHelper.cs 100.00% <100.00%> (ø)
...ngServer.UnitTests/BicepCompletionProviderTests.cs 100.00% <100.00%> (ø)
...p.LangServer/Completions/BicepCompletionContext.cs 96.89% <100.00%> (+0.53%) ⬆️
...ep.LangServer/Completions/CompletionItemBuilder.cs 92.59% <100.00%> (+0.28%) ⬆️
... and 2 more

@alex-frankel
Copy link
Collaborator

is it possible to get completions to show up on the first ', so if I am here, I would expect completions to show up:

module mod ''

However, I have to manually press ctrl + space in order to see the list.

@TSunny007
Copy link
Contributor Author

TSunny007 commented Dec 30, 2020

@alex-frankel , I noticed that too, will touch bases with team in the new year to figure out how to do that.
(Update 01.09-21) This should be the use experience now.

@TSunny007 TSunny007 linked an issue Jan 4, 2021 that may be closed by this pull request
@majastrz
Copy link
Member

majastrz commented Jan 4, 2021

Isn't this a bug in the completion item contents? Shouldn't completions always offer the canonical representation which would be ./main.bicep?


In reply to: 751831009 [](ancestors = 751831009)

@majastrz
Copy link
Member

majastrz commented Jan 4, 2021

If not let's grab some time to discuss


In reply to: 754205405 [](ancestors = 754205405,751831009)

Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

Added some comments

@majastrz
Copy link
Member

I just pulled your branch btw. Starting from this state:
image

Committing the subs/ completion produces the following without triggering the next segment:
image

@majastrz
Copy link
Member

The other thing I found was that I can't just trigger completions by typing the beginning of a folder name. For example, here typing s should have shown "subs/" completion:
image

And pressing ctrl+space in that state produces this:
image

@TSunny007 TSunny007 requested a review from majastrz January 28, 2021 00:16
Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

This is awesome!

@majastrz
Copy link
Member

One minor thing I found was that the file completions include the current module as well. So just blindly accepting completions led me to this :)
image

@TSunny007 TSunny007 merged commit 7207ddf into main Jan 28, 2021
@TSunny007 TSunny007 deleted the tsunkaraneni/ModuleFileCompletion branch January 28, 2021 18:24
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.

Intellisense for module file paths
5 participants