Fix when clauses for new interactive cell shortcuts #13273
Conversation
joyceerhl
requested review from
DavidKutu,
DonJayamanne,
IanMatthewHuff,
greazer and
rchiodo
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov Report
@@ Coverage Diff @@
## master #13273 +/- ##
==========================================
- Coverage 59.80% 59.78% -0.03%
==========================================
Files 670 670
Lines 37183 37183
Branches 5265 5265
==========================================
- Hits 22237 22228 -9
- Misses 13818 13824 +6
- Partials 1128 1131 +3
Continue to review full report at Codecov.
|
| "title": "%python.command.python.datascience.insertCellBelowPosition.title%", | ||
| "category": "Python", | ||
| "when": "python.datascience.hascodecells && editorFocus && editorLangId == python && python.datascience.featureenabled" | ||
| "when": "python.datascience.hascodecells && python.datascience.featureenabled && !notebookEditorFocused" |
There was a problem hiding this comment.
Don't you also need editorFocus && editorLangId == python
So that we display these commands only for python files?
There was a problem hiding this comment.
Yeah I saw that that's what you recommended in #12529, but it turns out that:
editorFocusdoesn't work at all for some reason. It completely hides those commands. Possibly codelenses interfering? I'm not totally sure what's going on there. But it doesn't do what we want. This was why the commands weren't showing up for @greazer during demo earlier today.- The
python.datascience.hascodecellscontext key is equivalent toeditorLangId == python, because we only sethascodecellsifactiveEditor.document.languageId === PYTHON_LANGUAGE:
vscode-python/src/client/datascience/datascience.ts
Lines 85 to 88 in 30af20a
| ); | ||
| export const addCellBelowCommandTitle = localize('DataScience.addCellBelowCommandTitle', 'Add cell'); | ||
| export const debugCellCommandTitle = localize('DataScience.debugCellCommandTitle', 'Debug cell'); | ||
| export const debugCellCommandTitle = localize('DataScience.debugCellCommandTitle', 'Debug Cell'); |
There was a problem hiding this comment.
Looks like the command above should be changed to match up.
There was a problem hiding this comment.
Is the Add Cell codelens even supposed to be showing up anymore?
There was a problem hiding this comment.
I guess you could check the telemetry. It's still a command though.
| "DataScience.jupyterDataRateExceeded": "Cannot view variable because data rate exceeded. Please restart your server with a higher data rate limit. For example, --NotebookApp.iopub_data_rate_limit=10000000000.0", | ||
| "DataScience.addCellBelowCommandTitle": "Add cell", | ||
| "DataScience.debugCellCommandTitle": "Debug cell", | ||
| "DataScience.debugCellCommandTitle": "Debug Cell", |
There was a problem hiding this comment.
Should all of the 'cell' commands have 'Cell' instead of 'cell'? There's others.
There was a problem hiding this comment.
Agreed, we have localize('DataScience.addCellBelowCommandTitle', 'Add cell');, shouldn't this be Add Cell
FYI - in VS Code, they are capatalized, i.e. i'd make this change everywhere.
There was a problem hiding this comment.
The rest of them don't even show up, I think we decided at some point last year not to show those codelenses. Can we remove them altogether?
There was a problem hiding this comment.
Oops just saw Rich's followup above.
|
Just make the names consistent, either 'cell' or 'Cell' |
|
I want to make sure I address the localization capitalization everywhere and I don't want to wait for CI to run all over again, so I'm gonna merge this one (since the Debug Cell codelens is the only one that is user-visible in the interactive python file experience). |
6 participants
For #13271
Kinda weird that the
editorFocuscontext key seems to break things. VSCode docs suggest that they're responsible for setting that key.editorLangId == pythonis harmless. Still, I'm dropping them for now as what I have here should be sufficient to ensure correct behavior.package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).