Make prefill in quick open configurable by ColCh · Pull Request #41586 · microsoft/vscode

ColCh · 2018-01-14T01:54:08Z

Close #41725

Demo: https://streamable.com/psso1

msftclas · 2018-01-14T01:54:18Z

All committers have signed the CLA.

usernamehw · 2018-01-14T07:22:23Z

Have you tried to invoke quick pick | go to symbol | command palette | ... when no editor is opened?

Cannot read property 'getControl' of null

Cannot read property 'getControl' of undefined

You've already made similar PR in Atom => and you hid functionality under configuration and turned it off by default. What is different now?

Multi-line: in your Atom PR when having multi-line selection you don't prefill the input, right? Seems more logical to me to skip multi-line selections here too...

ColCh · 2018-01-14T11:46:42Z

Have you tried to invoke quick pick | go to symbol | command palette | ... when no editor is opened?

wow thank you! didn't expected this case. Fixed

You've already made similar PR in Atom => and you hid functionality under configuration and turned it off by default. What is different now?

That behaviour was requested by member of Atom. Personally, I think that this feature should be turned ON by default and be unconfigurable. What do you think?

Webstorm has this feature as well. And AFAIK there is no config to turn it off.

Hm... I think it will be useful to control this feature

I will create 2 config entries:

to turn it ON/OFF
to control which prefixes to prefill from selection (it's for symbols and other prefixes I don't know)

Multi-line: in your Atom PR when having multi-line selection you don't prefill the input, right? Seems more logical to me to skip multi-line selections here too...

Agreed. Skipped that too. Fixed. Thank you!

usernamehw · 2018-01-14T12:36:40Z

src/vs/workbench/browser/parts/quickopen/quickOpenController.ts

+	}
+
+	private shouldPrefill(prefix?: string): boolean {
+		const isTurnedOn = this.configurationService.getValue('workbench.quickOpen.prefillFromSelection') as string;


This is boolean, right?

oh yes my bad

fixed

usernamehw · 2018-01-14T12:39:30Z

src/vs/workbench/browser/parts/quickopen/quickOpenController.ts

+
+		const prefillsPrefix = prefixes.indexOf(prefix) !== -1;
+
+		return prefillsPrefix;


Could've been one line?
return prefixes.indexOf(prefix) !== -1;

yeah

corrected to be one line

usernamehw · 2018-01-14T12:40:58Z

src/vs/workbench/electron-browser/main.contribution.ts

+				'type': ['string', 'null']
+			},
+			'default': [
+				null,


Not obvious for what null stands for.

null prefix is in file quick open

Then ... what for empty string?

it seems that empty string has no reason to be here

removed it

I thought maybe it would be better to remove null from config and use empty string (since empty string is the value in quick picker associated with go to file) - map it to null in code.

bpasero · 2018-01-14T17:49:55Z

@ColCh I see two possible solutions:

we have a setting for this (disabled by default)
we introduce a new command that opens quick open with the selected text of the editor

My reason why I would never want this as default is that our quick open includes a list of recently opened editors in order of their appearance. This list would not show up anymore as soon as some text is prefilled based on the selection in the editor.

Btw we have code that gets selection from the editor for the "Open Symbol in Workspace" quick open handler already, check out: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/search/electron-browser/search.contribution.ts#L268

ColCh · 2018-01-20T13:53:26Z

@bpasero thanks for suggestions and link to code

Ok, seems reasonable to turn on this feature only for search for all symbols, which is # prefix. This also will preserve current behaviour for prefill.

Open Symbol in Workspace is cool, but it prefills on ShowAllSymbolsAction, not counting all other quick open actions. Also, it selects text in quick open window, which was prefilled.

Feature implemented in current PR prefills on QuickOpen level, which stands separately.

I will remove prefill logic from ShowAllSymbolsAction and add selection of prefilled value.

* master: implement closeEditors properly when passing in editors

ColCh · 2018-01-20T13:55:39Z

Done, please check again

@groupId

* master: (280 commits) fix issue with useAltAsMultiSelectModifier Log ripgrep cmd from quickopen to logservice Switch search output panel to logservice Use absolute paths for webview core resources Explicitly hide TS version status when in ts/js file with unknown scheme First attempt of search caching. Pick up latest TS insiders 2018-01-19. Merged in translations from Transifex. Avoid double-counting same settings search query Use latest emmet helper that has fixes microsoft#33818 Remove settings search @groupId filters - since group ids can now be extension guids, and nobody uses this anyway backups - make sure to use "utf8" as encoding and not "utf-8" Catch any errors that onLineData users throw use uuid as extensions configuration id fix format on save problem that occurs when a provider returns no edits fix microsoft#41724 Fix microsoft#41868 some menu polish for issue reporter Fix microsoft#40260 Simplify edits handling in TextModelTokens ...

ColCh · 2018-01-21T21:37:23Z

Build is failing on Travis, but it seems that I can't clearly understand why it's failing...

ColCh · 2018-02-01T13:12:21Z

@bpasero ping?

bpasero · 2018-02-01T14:09:05Z

This change is not what I was expecting. I think I was expecting either a setting for just quick open (Cmd+P) or just a new command that would do this (I think a new command would be better).

ColCh · 2018-02-01T14:45:59Z

Thank you for your thoughts

I think I was expecting either a setting for just quick open (Cmd+P)

This will introduce same logic as symbol search across the project with different implementation. I just think that having single place of handing input prefill is better, isn't it?

a new command that would do this (I think a new command would be better)

Hm, interesting proposal. You mean, one command will open command menu with defined prefix (quick open, symbol search, other ...) and will append selected text, and another command, which will just open command menu without prefill (like now) ?

Ok, I will code this. AFAIK only symbol search across the project now prefills selected text?

Any other points to consider?

bpasero · 2018-02-01T14:51:18Z

@ColCh yeah we could add a new quick open command similar to Cmd+P today that puts the selection in. If there is any code to share between open symbols and open file, that would be good.

bpasero · 2018-02-07T11:06:43Z

@ColCh you could also do this from an extension, see my comment in #10413 (comment)

ColCh · 2018-02-07T21:22:21Z

@bpasero ooh thank you so much for that snippet! It seems that this functionality should not exist in core, but in extension, right?

bpasero · 2018-02-08T05:48:32Z

@ColCh yeah let's start with an extension 👍

Prefill quick open with selection

7d1ed7d

remove unused import

bd4a8b7

ColCh added 4 commits January 14, 2018 13:03

Handle nulls, handle multiline

d4e251a

Add config entries for prefill, control which prefixes to prefill

7997d3a

correct type

dff8968

Add diez prefix

50856ea

usernamehw reviewed Jan 14, 2018

View reviewed changes

fix type

33096ff

usernamehw reviewed Jan 14, 2018

View reviewed changes

ColCh added 2 commits January 14, 2018 15:44

inline variable

ec592ab

remove empty string

700ea9a

bpasero self-assigned this Jan 14, 2018

bpasero added this to the On Deck milestone Jan 14, 2018

ColCh added 5 commits January 20, 2018 16:41

Use getSelectionSearchString

af8cced

Select prefilled text

dbc2972

Remove prefill from ShowAllSymbolsAction

358bbc4

Remove use of quickOpenService

94f27f6

Cast empty prefix to empty string

0071d8b

Merge branch 'master' into 10413-fill-quick-open-with-selection

5467abd

* master: implement closeEditors properly when passing in editors

Restrict prefill to only "all symbols search"

929c98a

ColCh changed the title ~~Prefill quick open with selection~~ Make prefill in quick open configurable Jan 20, 2018

bpasero removed this from the On Deck milestone Jan 21, 2018

bpasero modified the milestone: Backlog Jan 21, 2018

ghost mentioned this pull request Jan 21, 2018

[monaco] bind ILogService to NullLogService by default #41927

Merged

bpasero added this to the On Deck milestone Feb 5, 2018

bpasero closed this Feb 8, 2018

github-actions bot locked and limited conversation to collaborators Mar 27, 2020


		const prefillsPrefix = prefixes.indexOf(prefix) !== -1;

		return prefillsPrefix;

Conversation

ColCh commented Jan 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msftclas commented Jan 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

usernamehw commented Jan 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ColCh commented Jan 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bpasero commented Jan 14, 2018

Uh oh!

ColCh commented Jan 20, 2018

Uh oh!

ColCh commented Jan 20, 2018

Uh oh!

ColCh commented Jan 21, 2018

Uh oh!

ColCh commented Feb 1, 2018

Uh oh!

bpasero commented Feb 1, 2018

Uh oh!

ColCh commented Feb 1, 2018

Uh oh!

bpasero commented Feb 1, 2018

Uh oh!

bpasero commented Feb 7, 2018

Uh oh!

ColCh commented Feb 7, 2018

Uh oh!

bpasero commented Feb 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ColCh commented Jan 14, 2018 •

edited

Loading

msftclas commented Jan 14, 2018 •

edited

Loading

usernamehw commented Jan 14, 2018 •

edited

Loading