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.

Sign up for GitHub

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

Jump to bottom

Duplicate context keys for input focus? #94168

Closed
jrieken opened this issue Apr 1, 2020 · 4 comments
Closed

Duplicate context keys for input focus? #94168

jrieken opened this issue Apr 1, 2020 · 4 comments
Assignees
@isidorn
Labels
context-keys debt Code quality issues
Milestone
April 2020

Comments

@jrieken
Copy link
Member

jrieken commented Apr 1, 2020

The editor has two focus states: 1) input focus (blinking cursor and 2) widget focus (focus is in find or peek). However, we have three context keys for this, just a few lines apart and my understanding is that 1) is duplicated

vscode/src/vs/editor/common/editorContextKeys.ts

Lines 11 to 23 in 0c29fa3

/**
* A context key that is set when the editor's text has focus (cursor is blinking).
*/
export const editorTextFocus = new RawContextKey<boolean>('editorTextFocus', false);
/**
* A context key that is set when the editor's text or an editor's widget has focus.
*/
export const focus = new RawContextKey<boolean>('editorFocus', false);
/**
* A context key that is set when any editor input has focus (regular editor, repl input...).
*/
export const textInputFocus = new RawContextKey<boolean>('textInputFocus', false);

@jrieken
Copy link
Member Author

jrieken commented Apr 1, 2020

Starting with @isidorn because your name is on those lines

@isidorn
Copy link
Contributor

isidorn commented Apr 1, 2020 •
edited

@jrieken there is difference, you can see it here

textInpuFocus is true iff a focus is in any text of the editor
editorTextFocus is true iff a focus is in any text of a non simple widget editor
editorFocus is true iff a focus is on the editor widget - not necesserily in the text

Simple editor widgets are one liner editors like the debug console, scm input and so on.
So they are not duplicated. It is confusing yes, but I think we can not dump them now.

@jrieken
Copy link
Member Author

jrieken commented Apr 2, 2020 •
edited

😮 oh, I think that requires a little more jsdoc then better naming, e.g simpleEditorTextFocus. However, feel free to close this

@isidorn
Copy link
Contributor

isidorn commented Apr 2, 2020

Let's leave this open to improve jsdocs.

@isidorn isidorn added context-keys debt Code quality issues labels Apr 2, 2020
@isidorn isidorn added this to the April 2020 milestone Apr 2, 2020
@isidorn isidorn closed this as completed in e206fb5 Apr 9, 2020
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@isidorn isidorn

Labels
context-keys debt Code quality issues
Projects
None yet
Milestone
April 2020
Development

No branches or pull requests
2 participants
@jrieken @isidorn