Issue 12414 python interactive cell editing shortcuts #12529

earthastronaut · 2020-06-24T03:25:22Z

For #12414

…s multiple modules

* add method insertCell to CodeWatcher which inserts at current line * add method insertCellBelowPosition which inserts below current selection * link insertCellBelowPosition to command python.datascience.insertCellBelowPosition

* add method insertCellBelow to the CodeWatcher * link insertCellBelow to python.datascience.insertCellBelow This will insert a cell below the last cell of the current selection

* add method insertCellAbove to the CodeWatcher * link insertCellAbove to python.datascience.insertCellAbove * will insert cell above the top cell of the current selection

* add deleteCells method to CodeWatcher * link deleteCells to python.datascience.deleteCells

* add selectCell method to CodeWatcher * link selectCell to python.datascience.selectCell

* add selectCellContents method to CodeWatcher * link selectCellContents to python.datascience.selectCellContents

…r and active

* Works similar to excell extend selection by one cell above * Linked command to python.datascience.extendSelectionByCellAbove

* Works similar to excell extend selection by one cell below * Linked command to python.datascience.extendSelectionByCellBelow

* Command to take selected cells and move them one cell up * linked command to python.datascience.moveCellsUp

* Moves selected cells one cell down * Linked to python.datascience.moveCellsDown

ghost · 2020-06-24T03:25:35Z

All CLA requirements met.

earthastronaut · 2020-06-24T03:34:03Z

👋 @greazer Here are changes I made for the keyboard editing shortcuts on the python interactive cells. There are a few check points on the PR that I'm not sure about.

Has sufficient logging. -- what is "sufficient"? I didn't see any logging in the code I was editing so I didn't have an archetype for what is expected.
Has telemetry for enhancements. -- I see some of the telemetry code. Is there a particular example I could emulate?
Test plan is updated as appropriate. -- What needs to change for this document? Do there need to be new interactive tests?
The wiki is updated with any design decisions/details. -- Is this an external wiki? what is needed here?

* command will use selected cell and if cell_type is not markdown will convert to markdown * linked to python.datascience.changeCellToMarkdown

greazer · 2020-06-30T01:21:27Z

@earthastronaut Thanks for doing this! We will take a look and let you know next steps!

package.json

src/client/datascience/editor-integration/codeLensFactory.ts

rchiodo

🕐

src/client/datascience/editor-integration/codewatcher.ts

rchiodo · 2020-06-30T17:00:39Z

To answer your questions:

Has sufficient logging. -- what is "sufficient"? I didn't see any logging in the code I was editing so I didn't have an archetype for what is expected.

We log just about every operation but it's probably not necessary for the changes you made. Unless they are complex enough that they may fail in the real world. I think we can live without it for now though.

Has telemetry for enhancements. -- I see some of the telemetry code. Is there a particular example I could emulate?

I think you should add telemetry for each new command you added. Follow the pattern for the other commands in the codewatcher.ts file.

Test plan is updated as appropriate. -- What needs to change for this document? Do there need to be new interactive tests?

Right now I don't think we need to add these changes to the test plan. (https://github.com/microsoft/vscode-python/blob/master/.github/test_plan.md). We generally use the test plan for manual testing and only higher order things. Since we have automated tests, that should be sufficient.

The wiki is updated with any design decisions/details. -- Is this an external wiki? what is needed here?

Our wiki is here: https://github.com/microsoft/vscode-python/wiki. We don't generally put every piece of code on it though as it'd be too much to keep up to date.

* command will use selected cell and if cell_type is not code then it'll convert to code * linked to python.datascience.changeCellToCode

revoking review

rchiodo

🕐

rchiodo · 2020-07-23T23:19:06Z

@earthastronaut just wanted to double check you weren't waiting for more feedback. The comments I made about some of the async functions don't all seem to have been addressed yet.

earthastronaut · 2020-07-27T20:51:47Z

Hi @rchiodo, yes will address them in the next few days. I was afk the last week and half.

earthastronaut · 2020-07-31T16:56:41Z

@rchiodo what is the standard for handling these conflicts? Is a rebase or merge preferred?

I started a rebase but am struggling with a change that's unrelated to mine that seems to have broken.

joyceerhl · 2020-07-31T17:26:28Z

Hey @earthastronaut, Rich is OOF this week. I believe merge is preferred. Could you try that and let me know if the merge conflict persists? i.e. after setting this repo as upstream, run git pull upstream master.

earthastronaut · 2020-07-31T17:49:13Z

Thanks @joyceerhl merge worked well. Just needed to combine a few changes from master into mine.

After merging master, my workspace is highlighting a warning with "categories" within the package.json file. Specifically, it's highlighting "Data Science", "Machine Learning", "Notebook" as invalid values within "categories". Looks related to a change by @DonJayamanne earlier this month and probably is intentional without need to worry.

joyceerhl · 2020-07-31T17:58:20Z

Apologies, I think you may need to delete these lines in codewatcher.ts: https://github.com/earthastronaut/vscode-python/blob/02cc7a5936735c994b9dd5cef59a0b6b347799a6/src/client/datascience/editor-integration/codewatcher.ts#L102-L104 Looks like the linter moved that to the top of the class above the private properties, so now there are two declarations of codeLensUpdated.

Resolved conflicts by merging master changes into branch changes manually to keep all new master changes. Conflicts: package.nls.json src/client/common/application/commands.ts src/client/datascience/editor-integration/codewatcher.ts

earthastronaut · 2020-07-31T18:02:50Z

Thanks @joyceerhl! I thought I pushed those changes up but they should be up now.

codecov-commenter · 2020-07-31T18:10:32Z

Codecov Report

Merging #12529 into master will increase coverage by 0.15%.
The diff coverage is 71.06%.

@@            Coverage Diff             @@
##           master   #12529      +/-   ##
==========================================
+ Coverage   59.63%   59.79%   +0.15%     
==========================================
  Files         670      670              
  Lines       36823    37172     +349     
  Branches     5188     5263      +75     
==========================================
+ Hits        21960    22226     +266     
- Misses      13770    13816      +46     
- Partials     1093     1130      +37

Impacted Files	Coverage Δ
src/client/datascience/cellMatcher.ts	`79.54% <ø> (ø)`
src/client/datascience/types.ts	`100.00% <ø> (ø)`
src/client/telemetry/index.ts	`82.35% <ø> (ø)`
src/client/datascience/commands/commandRegistry.ts	`34.53% <25.53%> (+0.74%)`	⬆️
...ient/datascience/editor-integration/codewatcher.ts	`70.62% <74.67%> (+11.60%)`	⬆️
src/client/datascience/cellFactory.ts	`64.28% <100.00%> (ø)`
src/client/datascience/constants.ts	`99.78% <100.00%> (+0.01%)`	⬆️
.../datascience/editor-integration/codeLensFactory.ts	`46.19% <100.00%> (+1.69%)`	⬆️
src/client/common/utils/platform.ts	`64.70% <0.00%> (-11.77%)`	⬇️
src/client/datascience/crossProcessLock.ts	`79.41% <0.00%> (-11.77%)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30b3035...716200b. Read the comment docs.

DonJayamanne · 2020-07-31T20:26:23Z

package.json

+                    "command": "python.datascience.selectCellContents",
+                    "title": "%python.command.python.datascience.selectCellContents.title%",
+                    "category": "Python",
+                    "when": "python.datascience.hascodecells && python.datascience.featureenabled && python.datascience.ispythonornativeactive"


This ispythonornativeactive will ensure this command is available in python files and notebook.
However in notebooks it does nothing. If this purpose of this PR is to add commnds for interactive window, then we shouldn't use this condition, i.e. it should be done only when a python file is active, e.,g. python.datascience.hascodecells && editorFocus && editorLangId == python && python.datascience.featureenabled

I.e. we need to ensure these commands are only displayed when we are in a python file with cells.

Thanks @DonJayamanne. I pushed up a change for that. Should that also be the same for runcurrentcell or runcurrentcelladvance? Those are what I modeled my choices after.

sonarqubecloud · 2020-08-01T15:34:36Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
0.0% Duplication

rchiodo

greazer · 2020-08-07T21:54:38Z

@earthastronaut, just want to thank you more personally for the effort you made here to get this in. We really appreciate outside contributions and this one, in particular, was one that is close to my heart. See, I really, really think the interactive experience has a lot of potential to not just appeal to software engineers who find themselves doing data science activities (as it's original intent was), but also has potential for establishing a more flexible, maintainable, and productive way of doing any work that is currently being done in traditional notebooks.

These commands you added for us (and others who requested them) continue to blur the line between what notebook and script development is. Finally, I took the step to add the ability to navigate from cell to cell (also requested) and connect ALL of the new commands to a default set of keyboard bindings. See PR #13334.

Thanks again!

earthastronaut · 2020-08-08T00:08:42Z

@greazer thank you so much for your words! That's fantastic you added the additional navigation. I'm excited to see if this finds traction in the world because I definitely agree with your points about flexible, maintainable and productive. I've started using the functions in my own coding and am loving it.

earthastronaut added 14 commits June 23, 2020 21:04

refactored ICellRange definition to type.ts since it's now used acros…

8f1fd7b

…s multiple modules

add command insertCellBelowPosition

fd5e865

* add method insertCell to CodeWatcher which inserts at current line * add method insertCellBelowPosition which inserts below current selection * link insertCellBelowPosition to command python.datascience.insertCellBelowPosition

add command insertCellBelow

127e2df

* add method insertCellBelow to the CodeWatcher * link insertCellBelow to python.datascience.insertCellBelow This will insert a cell below the last cell of the current selection

add command insertCellAbove

38fad86

* add method insertCellAbove to the CodeWatcher * link insertCellAbove to python.datascience.insertCellAbove * will insert cell above the top cell of the current selection

add deleteCells command

5afe4cc

* add deleteCells method to CodeWatcher * link deleteCells to python.datascience.deleteCells

add selectCell command

dd846c1

* add selectCell method to CodeWatcher * link selectCell to python.datascience.selectCell

add selectCellContents command

cfeccbb

* add selectCellContents method to CodeWatcher * link selectCellContents to python.datascience.selectCellContents

modify selectCell to maintain previous orientation of selection ancho…

500fe5a

…r and active

fix detection of anchor and active position direction

cf5e404

add extendSelectionByCellAbove

aa39444

* Works similar to excell extend selection by one cell above * Linked command to python.datascience.extendSelectionByCellAbove

add extendSelectionByCellBelow

9a073aa

* Works similar to excell extend selection by one cell below * Linked command to python.datascience.extendSelectionByCellBelow

add moveCellsUp

4a95409

* Command to take selected cells and move them one cell up * linked command to python.datascience.moveCellsUp

add moveCellsDown

95fc70d

* Moves selected cells one cell down * Linked to python.datascience.moveCellsDown

added news file for enhancement changes being added

cc37494

add changeCellToMarkdown

1f73555

* command will use selected cell and if cell_type is not markdown will convert to markdown * linked to python.datascience.changeCellToMarkdown

greazer requested review from rchiodo, DonJayamanne, IanMatthewHuff and joyceerhl and removed request for joyceerhl June 30, 2020 00:58

rchiodo reviewed Jun 30, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

rchiodo reviewed Jun 30, 2020

View reviewed changes

src/client/datascience/editor-integration/codeLensFactory.ts Outdated Show resolved Hide resolved

rchiodo previously requested changes Jun 30, 2020

View reviewed changes

rchiodo added the external contributor label Jun 30, 2020

rchiodo reviewed Jun 30, 2020

View reviewed changes

src/client/datascience/editor-integration/codewatcher.ts Outdated Show resolved Hide resolved

add changeCellToCode

e62137a

* command will use selected cell and if cell_type is not code then it'll convert to code * linked to python.datascience.changeCellToCode

rchiodo requested changes Jul 13, 2020

View reviewed changes

clean up unnecessary Promise.resolve()

4fdd64c

earthastronaut added 2 commits July 30, 2020 20:16

refactor unnecessary async functions to sync functions

9876718

added unit tests for move cells down

0b83e38

earthastronaut force-pushed the issue-12414-python-interactive-cell-shortcuts branch from 979b507 to 02cc7a5 Compare July 31, 2020 17:42

earthastronaut force-pushed the issue-12414-python-interactive-cell-shortcuts branch from 02cc7a5 to 6ac5741 Compare July 31, 2020 18:01

DonJayamanne requested changes Jul 31, 2020

View reviewed changes

change the when for commands to display when the python file has cells

716200b

joyceerhl requested review from DonJayamanne and rchiodo August 3, 2020 16:18

rchiodo approved these changes Aug 3, 2020

View reviewed changes

DonJayamanne approved these changes Aug 3, 2020

View reviewed changes

rchiodo merged commit f001bd1 into microsoft:master Aug 3, 2020

joyceerhl mentioned this pull request Aug 5, 2020

Fix when clauses for new interactive cell shortcuts #13273

Merged

10 tasks

earthastronaut deleted the issue-12414-python-interactive-cell-shortcuts branch August 6, 2020 21:54

luabud mentioned this pull request Aug 13, 2020

[Python] Document new cell commands microsoft/vscode-docs#3940

Closed

joyceerhl mentioned this pull request Nov 11, 2020

New interactive cell shortcuts don't show up when they should microsoft/vscode-jupyter#709

Closed

Issue 12414 python interactive cell editing shortcuts #12529

Issue 12414 python interactive cell editing shortcuts #12529

Uh oh!

Conversation

earthastronaut commented Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jun 24, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

earthastronaut commented Jun 24, 2020

Uh oh!

greazer commented Jun 30, 2020

Uh oh!

Uh oh!

Uh oh!

rchiodo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rchiodo commented Jun 30, 2020

Uh oh!

rchiodo left a comment

Choose a reason for hiding this comment

Uh oh!

rchiodo commented Jul 23, 2020

Uh oh!

earthastronaut commented Jul 27, 2020

Uh oh!

earthastronaut commented Jul 31, 2020

Uh oh!

joyceerhl commented Jul 31, 2020

Uh oh!

earthastronaut commented Jul 31, 2020

Uh oh!

joyceerhl commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

earthastronaut commented Jul 31, 2020

Uh oh!

codecov-commenter commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DonJayamanne Jul 31, 2020

Choose a reason for hiding this comment

Uh oh!

DonJayamanne Jul 31, 2020

Choose a reason for hiding this comment

Uh oh!

earthastronaut Aug 1, 2020

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Aug 1, 2020

Uh oh!

rchiodo left a comment

Choose a reason for hiding this comment

Uh oh!

greazer commented Aug 7, 2020

Uh oh!

earthastronaut commented Aug 8, 2020

Uh oh!

Uh oh!

earthastronaut commented Jun 24, 2020 •

edited

Loading

ghost commented Jun 24, 2020 •

edited by ghost

Loading

joyceerhl commented Jul 31, 2020 •

edited

Loading

codecov-commenter commented Jul 31, 2020 •

edited

Loading