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

Refactor jump-to functionality, remove unused code #423

Merged
merged 18 commits into from
Dec 13, 2020
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/job.test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ env:

LINKED_EXTENSIONS: >-
packages/lsp-ws-connection
packages/jupyterlab-go-to-definition
packages/code-jumpers
packages/completion-theme
packages/theme-vscode
packages/theme-material
Expand Down Expand Up @@ -148,7 +148,7 @@ jobs:
run: jlpm test

# js_cov_packages:
# - jupyterlab-go-to-definition
# - code-jumpers
# - jupyterlab-lsp

#- task: PublishTestResults@2
Expand Down
28 changes: 18 additions & 10 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@

- features

- The virtual document folder can be configure with `JP_LSP_VIRTUAL_DIR` or
`LanguageServerManager.virtual_documents_dir`. Its default value is kept
- the virtual documents' folder can be configured with `JP_LSP_VIRTUAL_DIR` or
`LanguageServerManager.virtual_documents_dir`; its default value is kept
unchanged: _contents.root_dir_ / `.virtual_documents` ([#416])

[#416]: https://github.com/krassowski/jupyterlab-lsp/issues/416

### `@krassowski/jupyterlab-lsp 2.0.9` (???)
### `@krassowski/jupyterlab-lsp 2.1.0` (???)

- features

- added "click to jump" functionality (by default using <kbd>Ctrl</kbd> modifier) ([#423])
- added "jump back" command, by default activated with <kbd>Ctrl</kbd> + <kbd>o</kbd> ([#423])

- bug fixes

Expand All @@ -19,14 +24,17 @@

[#403]: https://github.com/krassowski/jupyterlab-lsp/issues/403

### `@krassowski/jupyterlab_go_to_definition 2.0.0` (???)
### `@krassowski/code-jumpers 1.0.0` (???)

- features

- breaking change: renames `uri` to `contents_path` to help avoid programmer issues
- breaking changes
- split away from `@krassowski/jupyterlab_go_to_definition`, renamed to `@krassowski/code-jumpers` ([#423]):
- removed unused code
- refactored history operations to track files and always use global location data
- renamed `uri` to `contents_path` to help avoid programmer issues
with characters requiring URI encoding ([#406])

[#406]: https://github.com/krassowski/jupyterlab-lsp/pull/406
[#423]: https://github.com/krassowski/jupyterlab-lsp/pull/423

### `@krassowski/jupyterlab-lsp 2.0.8` (2020-10-25)

Expand Down Expand Up @@ -119,15 +127,15 @@
- rename operation status reporting got improved ([#318])
- replaced the generic status icons with code check icon (coloured differently according to the status) ([#318])
- added icons for all the features and their commands ([#318])
- refactored the codebase with a new architecture which allows dynamic feature, document widget adapter, and code editor registration ([#318])
- refactored the codebase with a new architecture which allows dynamic features, document widget adapter, and code editor registration ([#318])
- the document in the connections list in the statusbar popover are now represented by easy-to-understand DocumentLocator (breadcrumbs) widget rather than an internal id ([bacc006])
- syntax highlighting mode is adjusted to the language with the majority of the code in an editor ([#319])
- copy diagnostics message and filter diagnostics from context menu of Diagnostic Panel ([#330])

- bug fixes

- path-autocompletion issues were resolved upstream an this release adopts these changes
- missing caret and document connection icons were restored in the statusbar popover ([#318])
- path-autocompletion issues were resolved upstream and this release adopts these changes
- the missing caret and document connection icons were restored in the statusbar popover ([#318])
- pressing "Cancel" rename during rename now correctly aborts the rename operation ([#318])
- when a language server for a foreign document is not available an explanation is displayed (rather than the "Connecting..." status as before) ([4e5b2ad])
- when jump target is not found a message is now shown instead of raising an exception ([00448d0])
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ over the underlined code to see a more detailed message

### Jump to Definition

Use the context menu entries to jump to definitions
Use the context menu entry, or <kbd>Ctrl</kbd> + :computer_mouse: to jump to definitions (customizable); use <kbd>Alt</kbd> + <kbd>o</kbd> to jump back

![jump](https://raw.githubusercontent.com/krassowski/jupyterlab-lsp/master/examples/screenshots/jump_to_definition.png)

Expand Down
17 changes: 17 additions & 0 deletions atest/05_Features/Jump.robot
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,23 @@ Python Jumps between Files
Wait Until Page Contains ANOTHER_CONSTANT
Capture Page Screenshot 10-jumped.png

Ctrl Click And Jumping Back Works
[Setup] Prepare File for Editing Python editor jump.py
${usage} = Set Variable a_variable
${sel} = Set Variable xpath:(//span[contains(@class, 'cm-variable')][contains(text(), '${usage}')])[last()]
Click Element ${sel}
${original} = Measure Cursor Position
Capture Page Screenshot 01-ready-to-jump.png
Click Element ${sel} modifier=CTRL
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You think 🍎 people will expect ⌘ ?

Copy link
Member Author

@krassowski krassowski Dec 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it seems so there was a failure on this. Should we mention it in readme, or is it typically obvious to 🍎 people?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting post: https://nuclide.io/blog/2017/02/27/Command-Click-You-Have-One-Job/
I forgot about the multi-cursor placement! Ah, this is difficult to settle. If we keep Ctrl as default it might break workflow for some users; at least there is a setting for this; though indeed many IDEs use Ctrl + click for jump, and I think that fewer use for placement of additional cursors - so that seems like a tie to me.

Capture Page Screenshot 02-jumped.png
Wait Until Keyword Succeeds 10 x 1 s Cursor Should Jump ${original}
${new} = Measure Cursor Position
Press Keys None ALT+o
Wait Until Keyword Succeeds 10 x 1 s Cursor Should Jump ${new}
${back} = Measure Cursor Position
Should Be Equal ${original} ${back}
[Teardown] Clean Up After Working With File jump.py

*** Keywords ***
Copy Files to Folder With Spaces
[Arguments] @{files}
Expand Down
4 changes: 4 additions & 0 deletions atest/examples/jump.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
a_variable = 1


a_variable
Binary file modified examples/screenshots/jump_to_definition.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@
"private": true,
"scripts": {
"bootstrap": "jlpm --no-optional --prefer-offline && lerna bootstrap && jlpm lint && jlpm clean && jlpm build",
"build": "jlpm build:schema && jlpm build:completion-theme && jlpm build:theme-material && jlpm build:theme-vscode && jlpm build:meta && jlpm build:ws",
"build": "jlpm build:schema && jlpm build:completion-theme && jlpm build:theme-material && jlpm build:theme-vscode && jlpm build:jump && jlpm build:meta && jlpm build:ws",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly would love to get rid of this command, as build:meta should do it all...

"build:schema": "lerna run build:schema --stream",
"build:meta": "lerna run build --stream --scope @krassowski/jupyterlab-lsp-metapackage",
"build:completion-theme": "lerna run build --stream --scope @krassowski/completion-theme",
"build:theme-vscode": "lerna run build --stream --scope @krassowski/theme-vscode",
"build:theme-material": "lerna run build --stream --scope @krassowski/theme-material",
"build:ws": "lerna run build --stream --scope lsp-ws-connection",
"build:jump": "lerna run build --stream --scope @krassowski/code-jumpers",
"watch": "lerna run --parallel watch",
"bundle": "lerna run --parallel bundle",
"clean": "lerna run --parallel clean",
Expand Down
3 changes: 3 additions & 0 deletions packages/code-jumpers/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Code Jumpers

Implementation underlying the jump to definition functionality in [JupyterLab-LSP](https://github.com/krassowski/jupyterlab-lsp).
59 changes: 59 additions & 0 deletions packages/code-jumpers/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
{
"name": "@krassowski/code-jumpers",
"version": "1.0.0",
"description": "Implementation underlying the jump to definition functionality in JupyterLab-LSP",
"keywords": [
"jupyter",
"jupyterlab",
"lsp",
"language-server-protocol"
],
"homepage": "https://github.com/krassowski/jupyterlab-lsp",
"bugs": {
"url": "https://github.com/krassowski/jupyterlab-lsp/issues"
},
"license": "BSD-3-Clause",
"author": "JupyterLab-LSP Development Team",
"files": [
"{lib,style,schema,src}/**/*.{d.ts,eot,gif,html,jpg,js,js.map,json,png,svg,woff2,ttf,css,json,ts,tsx}"
],
"main": "lib/index.js",
"types": "lib/index.d.ts",
"repository": {
"type": "git",
"url": "https://github.com/krassowski/jupyterlab-lsp.git"
},
"scripts": {
"build": "tsc -b",
"bundle": "npm pack .",
"clean": "rimraf lib",
"lab:link": "jupyter labextension link . --no-build"
},
"peerDependencies": {
"@jupyterlab/apputils": "~2.2.0",
"@jupyterlab/codeeditor": "~2.2.0",
"@jupyterlab/coreutils": "~4.2.0",
"@jupyterlab/docmanager": "~2.2.0",
"@jupyterlab/docregistry": "~2.2.0",
"@jupyterlab/fileeditor": "~2.2.0",
"@jupyterlab/notebook": "~2.2.0",
"@jupyterlab/observables": "~3.2.0"
},
"devDependencies": {
"@jupyterlab/apputils": "~2.2.0",
"@jupyterlab/codeeditor": "~2.2.0",
"@jupyterlab/coreutils": "~4.2.0",
"@jupyterlab/docmanager": "~2.2.0",
"@jupyterlab/docregistry": "~2.2.0",
"@jupyterlab/fileeditor": "~2.2.0",
"@jupyterlab/notebook": "~2.2.0",
"@jupyterlab/observables": "~3.2.0",
"@jupyterlab/testutils": "~2.2.0",
"rimraf": "^2.6.1",
"typescript": "~4.0.2",
"@babel/preset-env": "^7.4.3"
},
"jupyterlab": {
"extension": false
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IJumpPosition } from './jump';
import { IGlobalPosition } from './positions';
import { IModelDB, IObservableUndoableList } from '@jupyterlab/observables';
import { JSONValue } from '@lumino/coreutils';

Expand All @@ -24,12 +24,12 @@ export class JumpHistory {
}
}

store(position: IJumpPosition) {
store(position: IGlobalPosition) {
this.ensure_history_is_ready();
this.jump_history.push(JSON.stringify(position));
}

recollect(): IJumpPosition {
recollect(): IGlobalPosition {
this.ensure_history_is_ready();
if (this.jump_history.length === 0) {
return;
Expand All @@ -38,6 +38,6 @@ export class JumpHistory {
// being lazy here - undo addition instead of removal ;)
this.jump_history.undo();

return JSON.parse(last_position as string) as IJumpPosition;
return JSON.parse(last_position as string) as IGlobalPosition;
}
}
5 changes: 5 additions & 0 deletions packages/code-jumpers/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { CodeJumper } from './jumpers/jumper';
import { NotebookJumper } from './jumpers/notebook';
import { FileEditorJumper } from './jumpers/fileeditor';

export { CodeJumper, NotebookJumper, FileEditorJumper };
69 changes: 69 additions & 0 deletions packages/code-jumpers/src/jumpers/fileeditor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { FileEditor } from '@jupyterlab/fileeditor';
import { IGlobalPosition, ILocalPosition } from '../positions';
import { CodeJumper, jumpers } from './jumper';
import { JumpHistory } from '../history';
import { IDocumentManager } from '@jupyterlab/docmanager';
import { IDocumentWidget } from '@jupyterlab/docregistry';
import { CodeEditor } from '@jupyterlab/codeeditor';

export class FileEditorJumper extends CodeJumper {
editor: FileEditor;
widget: IDocumentWidget;

constructor(
editor_widget: IDocumentWidget<FileEditor>,
document_manager: IDocumentManager
) {
super();
this.widget = editor_widget;
this.document_manager = document_manager;
this.editor = editor_widget.content;
this.history = new JumpHistory(this.editor.model.modelDB);
}

get path() {
return this.widget.context.path;
}

get editors() {
return [this.editor.editor];
}

jump(jump_position: ILocalPosition) {
let { token } = jump_position;

// TODO: this is common
// place cursor in the line with the definition
let position = this.editor.editor.getPositionAt(token.offset);
this.editor.editor.setSelection({ start: position, end: position });
this.editor.editor.focus();
}

getOffset(position: CodeEditor.IPosition) {
return this.editor.editor.getOffsetAt(position);
}

getJumpPosition(position: CodeEditor.IPosition): ILocalPosition {
return {
token: {
offset: this.getOffset(position),
value: ''
},
index: 0
};
}

getCurrentPosition(): IGlobalPosition {
let position = this.editor.editor.getCursorPosition();
console.log('file path: ', this.editor.context.path);
return {
editor_index: null,
line: position.line,
column: position.column,
contents_path: this.editor.context.path,
is_symlink: false
};
}
}

jumpers.set('fileeditor', FileEditorJumper);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why side-effect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

legacy, keeping as is for now, but needs a refactor in future.

Loading