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

various fixes to make git and gitlens vscode extension work #6921

Merged
merged 20 commits into from
May 12, 2020
Merged

various fixes to make git and gitlens vscode extension work #6921

merged 20 commits into from
May 12, 2020

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Jan 20, 2020 •
edited
Loading

What it does

Please see individual commits, they reference relevant issues and description. Please ask if something is not clear i will try to clarify commits.

TODO:

  • add tests for exposed OpenerService Monaco APIs
  • add integration tests for keybinding regressions and bugs
  • fix integration tests
  • add missing commands, see https://github.com/eamodio/vscode-gitlens/blob/30e1d290e8f337d56596a030ff097eeabdd98eb7/src/constants.ts#L11
  • clicking on a file in repositories view opens an empty diff editor -> 465921d
    • It seems to be a bug in the editor preview widget, after pinning it works nicely.
  • no compare with previous revision in the navigator context menu because of preference proxies don't handle preferences from subtypes #6948
  • Uncaught command 'gitlens.diffWith' NOT known from markdownRenderer.ts:73 -> resolved by hooking OpenerService in Monaco
  • changing layout on welcome page trigger many overwrite dialog boxes: Update of preferences from VSCode plugin triggers multiple overwrite popup confirmation #6845 (resolved by [preferences] use text models to update content #7110)
  • clicking on source content layout in the welcome screen does nothing -> the command open handler was not handling Object as an argument, so string argument was spread to an array of characters
  • Error: No tree view with id gitlens.views.compare:gitlens when trying to change layouts from the welcome page -> extension issue No tree view id registered gitkraken/vscode-gitlens#966, the same in VS Code
  • root ERROR Failed to fetch children for 'gitlens.views.repositories:gitlens' Error: No tree view with id gitlens.views.repositories:gitlens while refreshing repositories view -> could not reproduce it after fixing other issues
  • editor navigation with keys to left and right does not work, key context for gitlens is not updated properly
    • quick pick cancellation was not handled properly (reproducible by trying to navigate with alt+left/right after using the more quick pick from gitlens hover)
    • valid keybindings were removed as collided without evaluating context/when closures (reproducible by trying to navigate with left/right always)
  • errors on the initial startup (reproducible by deleting ~/.theia/plugin-storage/global-state.json file) -> configuration for sub section was not properly cloned, see c4b2ff3
  • MonacoEditorModel backed up by resources from the plugin system are leaking as well as these resources

How to test

Review checklist

Reminder for reviewers

@akosyakov akosyakov added git issues related to git vscode issues related to VSCode compatibility labels Jan 20, 2020
@akosyakov akosyakov force-pushed the akosyakov/vscode-plugin-gitlens-4902 branch 3 times, most recently from 366d5ad to f3d9b50 Compare January 23, 2020 05:38
@akosyakov akosyakov self-assigned this Feb 3, 2020
@akosyakov akosyakov force-pushed the akosyakov/vscode-plugin-gitlens-4902 branch 2 times, most recently from ec85963 to 50e32d3 Compare February 9, 2020 08:37
@akosyakov akosyakov mentioned this pull request Feb 11, 2020
Merged
2 tasks
@akosyakov akosyakov changed the base branch from master to akosyakov/update-of-preferences-6845 February 12, 2020 08:12
@akosyakov akosyakov force-pushed the akosyakov/vscode-plugin-gitlens-4902 branch from 50e32d3 to a639d42 Compare February 12, 2020 08:29
@akosyakov akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch 3 times, most recently from a8da83d to 00535be Compare February 12, 2020 13:50
@akosyakov akosyakov force-pushed the akosyakov/vscode-plugin-gitlens-4902 branch from a639d42 to 304812b Compare February 12, 2020 14:18
@akosyakov akosyakov changed the base branch from akosyakov/update-of-preferences-6845 to master February 12, 2020 14:22
@akosyakov akosyakov force-pushed the akosyakov/vscode-plugin-gitlens-4902 branch 2 times, most recently from 69c8782 to 130db96 Compare February 12, 2020 15:15
@garethwhittaker garethwhittaker mentioned this pull request Feb 15, 2020
Closed
1 task
@akosyakov akosyakov force-pushed the akosyakov/vscode-plugin-gitlens-4902 branch 11 times, most recently from 6a68faf to 5ac1baf Compare February 25, 2020 14:25
akosyakov added 13 commits May 7, 2020 09:04
@akosyakov
[keybinding] don't exclude keybinding collided only by keys
fc4d2e8 
Keybindings are only collided when they belong to the same scope. It is not possible to detect without evaluating all keybindings which is expensive. Instead of we order keybindings according to scope semantics and evaluate in such order, the first one wins.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[vscode] fix eclipse-theia#6929, fix eclipse-theia#5764: handle quick…
c517df6 
… pick/input cancellation and closing properly

- resolve callback was never called if a user closed the widget
- token was never passed to the main side, so the widget was not hidden if an extension closes it

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] look up a configuration from cloned object, not clone function
eb00040 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
fix eclipse-theia#6948: handle nested default values by preference pr…
e04e391 
…oxies

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin][tree] better error formatting
a14b97a 
add a whitespace and put tree view id in quotes

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[gitlens] implement missing vscode commands
46d6d19 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
default to 100% as editor and editor preview height
5337f07 
It fixes the issue that the editor preview is empty sometimes.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] fix leaking FSResource
7eeb6b6 
Each resolve call should provide a new resource, resources should be cleaned up when a client dispose them.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
added keybindings regressions integration tests
aca281b 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] focus a requested view not a containing view container
57d186a 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] fixed update of quick pick items
f8c4e83 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] fixed tree view reveal
2f06b86 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] fix computing external icon urls
540f2c8 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the akosyakov/vscode-plugin-gitlens-4902 branch from fa07126 to 540f2c8 Compare May 7, 2020 09:37
@akosyakov
[plugin] support when closure for quick view palette
baf09dc 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
Copy link
Member Author

akosyakov commented May 7, 2020 •
edited
Loading

Wow, it turned out there are 2 substantial bugs in how we manage documents:

  • when code of synching active editor was copied, null value was lost, only undefined preserve, but it actually had different meaning, null means no active editor, undefined means no change, so active editor was working by luck so far
  • vscode.window.visibleTextEditors actually returns only visible, not all as we do

I will push a commit fixing it.

@akosyakov
[plugin] fix synching of visible editors and active
594f2d1 
- use null to indicate that there is no active, and undefined that there is no change, before logic was ignoring no active
- vscode.window.visibleTextEditors should return visible, not all editors

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
Copy link
Member Author

@azatsarynnyy Could you try latest changes please?

I left out:

  • jumping cursor, there is something wrong with how the editor decorations from VS Code extensions are installed
  • and rerendering view toolbar items, it is a generic issue with our view containers.

I will file follow-up issues for them. The rest should be fixed.

@azatsarynnyy
Copy link
Member

azatsarynnyy commented May 8, 2020 •
edited
Loading

GitLens Settings page doesn't display the images:

Interestingly for this case I have pictures in Gitpod:
Screenshot 2020-05-07 at 09 15 52

Where do you try it? Maybe it is related to how webviews are deployed in your environments.

@akosyakov I tried it running the browser example of vanilla Theia on my machine, localhost.

@azatsarynnyy
Copy link
Member

@azatsarynnyy Could you try latest changes please?

I left out:

  • jumping cursor, there is something wrong with how the editor decorations from VS Code extensions are installed
  • and rerendering view toolbar items, it is a generic issue with our view containers.

I will file follow-up issues for them. The rest should be fixed.

I re-checked and can confirm that the rest is fixed now. Thanks!

@akosyakov
Copy link
Member Author

I'm merging tomorrow morning If no one has objections.

@akosyakov akosyakov merged commit 362e8f1 into eclipse-theia:master May 12, 2020
@akosyakov akosyakov deleted the akosyakov/vscode-plugin-gitlens-4902 branch May 12, 2020 08:11
This was referenced May 12, 2020
Open
Closed
@akosyakov
Copy link
Member Author

@azatsarynnyy I filed #7795 and #7796 for remaining issues.

This was referenced May 12, 2020
Merged
Closed
@akosyakov akosyakov mentioned this pull request May 14, 2020
Closed
@sstone1 sstone1 mentioned this pull request Jun 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azatsarynnyy azatsarynnyy azatsarynnyy approved these changes

@paul-marechal paul-marechal Awaiting requested review from paul-marechal

@svenefftinge svenefftinge Awaiting requested review from svenefftinge

@vinokurig vinokurig Awaiting requested review from vinokurig

@AlexTugarev AlexTugarev Awaiting requested review from AlexTugarev

@vince-fugnitto vince-fugnitto Awaiting requested review from vince-fugnitto

@spoenemann spoenemann Awaiting requested review from spoenemann

@kittaakos kittaakos Awaiting requested review from kittaakos

Assignees

@akosyakov akosyakov

Labels
git issues related to git vscode issues related to VSCode compatibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vscode plugin gitlens not support
3 participants
@akosyakov @svenefftinge @azatsarynnyy