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: Use File Icon for suggestions #44860

Closed
octref opened this issue Mar 1, 2018 · 26 comments
Closed

Path Completion: Use File Icon for suggestions #44860

octref opened this issue Mar 1, 2018 · 26 comments
Assignees
Labels
feature-request Request for new features or functionality release-notes Release notes issues suggest IntelliSense, Auto Complete verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@octref
Copy link
Contributor

octref commented Mar 1, 2018

Whether it comes from internal or external icon theme.

For example, this isn't really nice

image

Would be great to show js/ts icons there.

For html path completion it would be much nicer too.

This would probably require changes in CompletionItemKind in LSP.

@mjbvz think you might be interested too.

@octref octref added the feature-request Request for new features or functionality label Mar 1, 2018
@mjbvz
Copy link
Collaborator

mjbvz commented Mar 1, 2018

Duplicate of #43204 which I closed as not worth the effort for js/ts. Html paths could be more interesting

@usernamehw
Copy link
Contributor

With modern module bundlers (like webpack) it's possible to import other files (.css, .png,...) from js. Icons would be helpful in that case too.

@dbaeumer
Copy link
Member

dbaeumer commented Mar 2, 2018

I am not sure what I should do with this. The CompletionItemKind has values for file and folder. There are no plans in the LSP to have CompletionItemKinds for different file types.

As @mjbvz pointed out he closed this request against TS.

@octref any objection to close this issue?

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Mar 2, 2018
@octref
Copy link
Contributor Author

octref commented Mar 2, 2018

@dbaeumer I do realize LSP isn't the right place to add support for this feature.
@jrieken I see #22206 and am wondering if you can provide any input. We have the dynamically generated icon for type color -- wondering if the same can be done for type file.

@jrieken
Copy link
Member

jrieken commented Mar 2, 2018

The reason for keeping the number of completion item kinds is low is to lower the burden of implementing the LSP. It's not nice to require an implementor to bring along 50+ icons. Custom, extension defined, icons isn't the answer neither because then e.g. Eclipse-icons will be used inside VS Code leading to an alien-look.

It's a fine line of finding an expressive set of common icons and we will never meet all demands but that's also the idea of it.

@octref
Copy link
Contributor Author

octref commented Mar 3, 2018

@jrieken Can we possibly make this a client-specific behavior. For example, when CompletionItemKind is file, read the filename in either label or textedit, match it against VS Code's matching rule for file icons, and use that icon for completion.

If other LSP clients don't have the file icon notion they can always provide one file icon.
It really helps to differentiate between the suggestion items. As @usernamehw said, Webpack allows you to import css/jpg/png/svg and whatever file Webpack's loader could resolve.

@jrieken
Copy link
Member

jrieken commented Mar 5, 2018

For example, when CompletionItemKind is file, read the filename in either label or textedit, match it against VS Code's matching rule for file icons, and use that icon for completion.

Hm, interesting idea I like. Maybe a little harder to magic-match than colors but we should give it a try

@jrieken jrieken added suggest IntelliSense, Auto Complete and removed info-needed Issue requires more information from poster labels Mar 5, 2018
@jrieken jrieken assigned jrieken and unassigned dbaeumer Mar 5, 2018
@jrieken jrieken added this to the October 2018 milestone Oct 8, 2018
@jrieken
Copy link
Member

jrieken commented Oct 8, 2018

@mjbvz For this to work, we would need to get the full path (or at least the name with its extension), e.g. as documentation label...

@svipas
Copy link
Contributor

svipas commented Oct 11, 2018

I don't even know how to express myself, @jrieken created tabCompletion -> 'on' and localityBonus. @octref is working on path completion icons. Guys, it's not even Christmas and I'm getting all my gifts. I'm sooo happy 😄

Sorry for offtopic 😄

@jrieken
Copy link
Member

jrieken commented Oct 11, 2018

The one challenge here is how file icons look together with symbols icon. The former can be themed the latter not so this has potential for ugliness...

@jrieken
Copy link
Member

jrieken commented Oct 16, 2018

We have a first version for this

screenshot 2018-10-16 at 09 48 43

@svipas
Copy link
Contributor

svipas commented Oct 16, 2018

@jrieken Looks very nice, amazing work!!! 🔥

I have several questions:

  1. It will adapt to the icon theme?
  2. It will impact the performance of suggestions rendering? I'm asking cuz' sometimes I see Loading...in suggestion widget.
  3. It will have a setting to enable/disable icons or it will be enabled by default to show icons?

My main concern is performance and the ability to use the same icons as selected in the icon theme.

@jrieken
Copy link
Member

jrieken commented Oct 16, 2018

Yeah, it will take the icons from the icon theme. When no icon theme is in use you get what you have today. Performance impact shouldn't be noticeable, we only render different icons. The Loading... comes from the language brain, computing suggestions.

oct-16-2018 11-22-21

@svipas
Copy link
Contributor

svipas commented Oct 16, 2018

Thanks, @jrieken, I really appreciate your work and all the information. 👍

@yzhang-gh
Copy link
Contributor

Looks great! One small problem, the folder icon feels not right 😅

image

@jrieken
Copy link
Member

jrieken commented Oct 16, 2018

Yeah, it still needs some tweaks with alignment and it doesn't respect folder icons yet. The challenge is that some themes don't have an icon for folders...

jrieken added a commit that referenced this issue Oct 16, 2018
@usernamehw
Copy link
Contributor

It seems to be aligned to top...
screenshot 72

And it's not working inside typescript import path completion... Is it intentional?

OS: Win10 x64 799bc78

@octref
Copy link
Contributor Author

octref commented Oct 17, 2018

I think for TS, they return import path without suffix and there is no way to guess the file icon from it.

@jrieken
Copy link
Member

jrieken commented Oct 18, 2018

And it's not working inside typescript import path completion

@usernamehw #44860 (comment)

It seems to be aligned to top...

Do you have a special config for the suggest font size?

jrieken added a commit that referenced this issue Oct 18, 2018
jrieken added a commit that referenced this issue Oct 18, 2018
@usernamehw
Copy link
Contributor

"editor.suggestLineHeight": 40,
"editor.suggestFontSize": 20,

@Astrantia
Copy link

@jrieken Can we add a configuration for this to enable/disable? Personally, I like this, but if someone dislikes it they will always be able to disable it from settings.

jrieken added a commit that referenced this issue Oct 19, 2018
@Astrantia
Copy link

@jrieken This is broken in the latest insiders. I only get icons for folders, but file icons don't work anymore.

@octref
Copy link
Contributor Author

octref commented Oct 23, 2018

@jrieken I'm in a TS file and trying to do require, but TS LS returns all filenames without the suffix. Is there any way TS could update their CompletionItem so it displays the nice icons?

image

@jrieken
Copy link
Member

jrieken commented Oct 24, 2018

@octref please read #44860 (comment)

@octref
Copy link
Contributor Author

octref commented Oct 24, 2018

@jrieken I see, I'll try to send a PR for TypeScript, so the CompletionItems have label with suffixes like main.ts, router.js but the textedit doesn't contain the labels.

@usernamehw
Copy link
Contributor

Icons are way too small now. Is it possible to have them the same size as before?:

Stable Insiders
before after

@jrieken jrieken added the verification-needed Verification of issue is requested label Oct 29, 2018
@miguelsolorio miguelsolorio added the verified Verification succeeded label Oct 31, 2018
@jrieken jrieken added the release-notes Release notes issues label Nov 2, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality release-notes Release notes issues suggest IntelliSense, Auto Complete verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

9 participants