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

path completion: support code completion inside an existing path until next whitespace #44814

Closed
aeschli opened this issue Mar 1, 2018 · 8 comments
Assignees
Labels
css-less-scss Issues and items concerning CSS,Less,SCSS styling verified Verification succeeded
Milestone

Comments

@aeschli
Copy link
Contributor

aeschli commented Mar 1, 2018

Testing #44460

Version 1.21.0-insider
Commit 9a04587
Date 2018-03-01T06:47:39.037Z

  • create foo.html, bar.html in a folder
  • in `foo.html' have
<a href="./boo.html"

and do code completion after b.

-> should show bar.html as a proposal. Accepting the proposal should replace /boo.html with ./bar.html

This would be consistent with other html completions, e.g. set the cursor inside href and pick hreflang

@octref
Copy link
Contributor

octref commented Mar 1, 2018

This would be consistent with other html completions, e.g. set the cursor inside href and pick hreflang

I had no idea of this feature. I'll look into supporting it for path completion for March.

@octref octref added this to the March 2018 milestone Mar 1, 2018
@octref
Copy link
Contributor

octref commented Mar 8, 2018

Actually that's a rather strange behavior that I don't think many other languages support.
Do people use it?

ts

json

I do see CSS supports it well. I think if we were to have this as a standard behavior of completions, other languages should have consistent completion behaviors with it too.

@octref
Copy link
Contributor

octref commented Mar 8, 2018

Looping in @mjbvz to get some insight from TS perspective.

@octref octref modified the milestones: March 2018, Backlog Mar 8, 2018
@aeschli aeschli added the css-less-scss Issues and items concerning CSS,Less,SCSS styling label Mar 9, 2018
@aeschli
Copy link
Contributor Author

aeschli commented Mar 9, 2018

It's replacement vs insertion which is currently different for each language.
It HTML it's consistently replacement, IMO we want to stick with it.

@aeschli aeschli modified the milestones: Backlog, March 2018 Mar 9, 2018
@octref
Copy link
Contributor

octref commented Mar 9, 2018

But I found this case to be very common too (echoed by @mjbvz):

Let's say I have a line

<script src="./vendor/foo/js/foo.js"></script>

And I have decided to move vendor to static folder, so the line needs to be changed to

<script src="./static/vendor/foo/js/foo.js"></script>

I wish I can move my cursor to after ./, do a completion, and choose static.

@aeschli
Copy link
Contributor Author

aeschli commented Mar 12, 2018

In Eclipse we had a modification key (Ctrl or Alt) to switch between insertion and replace mode on demand.
We don't have it in VSCode. The workaround is to add a space or complete before the '/'.

@octref
Copy link
Contributor

octref commented Mar 13, 2018

OK, so the feature become two things:

  1. Offer completions based on the path before cursor (if the value before cursor is ./b, use ./ as the path to get completion items (also with b as filter characters?)
  2. Instead of replacing everything towards the end of the attribute, use whitespace as a heuristic to calculate the replacement range, so it becomes the range from previous / position to either a whitespace or the end of attribute value.

1 is easy but for 2 I'm wondering how can I possibly deal with folder with whitespaces.

@aeschli
Copy link
Contributor Author

aeschli commented Mar 13, 2018

Yes, 1. is correct. The last segment has no meaning for the proposals. FIltering is always done in the UI, using the UI's heuristics (fuzzymatch, camelcase...).

Actually what we complete are URLs and URLs must not have spaces. So to be correct, we should URL encode the file name (so that spaces use become 20%).
Given that, extending the range until a whitespace is found makes sense.
Lets have test cases for all that.

@octref octref changed the title path completion: support code completion inside an existing path path completion: support code completion inside an existing path until next whitespace Mar 21, 2018
@ramya-rao-a ramya-rao-a added the verified Verification succeeded label Mar 30, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
css-less-scss Issues and items concerning CSS,Less,SCSS styling verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants