Add git to command palette #1243

tsabbir96 · 2023-05-15T12:28:50Z

An attempt to develop the feature requested at #1238

welcome · 2023-05-15T12:28:52Z

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

github-actions · 2023-05-15T12:29:02Z

👈 Launch a binder notebook on branch tsabbir96/jupyterlab-git/add-git-to-command-palette

fcollonval

You are on the right path.

In addition to the comments in the code, I must mention that the clone command is defined in a separate plugin; see https://github.com/jupyterlab/jupyterlab-git/blob/master/src/cloneCommand.ts#L20

src/index.ts

fcollonval · 2023-05-15T13:53:16Z

src/index.ts

+  // Add commands to the pallette
+  const command = CommandIDs.gitClone;
+
+  commands.addCommand(command, {


This will add a new command. But in this specific case we want to add to the palette existing commands. So if you want to test something, you will need to create a dummy command identifier as this one will clash with the existing command).

Understood, thanks for explaining!

fcollonval · 2023-05-15T13:54:46Z

src/index.ts

+  });
+  // Add the command to the command palette
+  const category = 'Git Example';
+  palette.addItem({ command, category });


This is good. You should do this after line 192 in which addCommands is executed (registering most of this extension commands).

tsabbir96 · 2023-05-16T08:15:58Z

@fcollonval Hi, I adjusted the changes that you requested. Looks like the PR is failing an Integration Test (UI Test). Here's the generated log:

Running 18 tests using 1 worker
··············F···

  1) tests/merge-commit.spec.ts:59:7 › Merge commit tests › should diff file after clicking ────────

    Error: proxy.waitForSelector: Target closed
    =========================== logs ===========================
    waiting for locator('.jp-git-diff-root') to be visible
    ============================================================

      69 |       .waitFor({ state: 'visible' });
      70 |
    > 71 |     expect(page.waitForSelector('.jp-git-diff-root')).toBeTruthy();
         |                 ^
      72 |   });
      73 |
      74 |   test('should revert merge commit', async ({ page }) => {

        at /home/runner/work/jupyterlab-git/jupyterlab-git/ui-tests/tests/merge-commit.spec.ts:71:[17](https://github.com/jupyterlab/jupyterlab-git/actions/runs/4788641195/jobs/8515773749?pr=1228#step:12:18)

    Test finished within timeout of 60000ms, but tearing down "context" ran out of time.
    Please allow more time for the test, since teardown is attributed towards the test timeout budget.

    attachment #1: video (video/webm) ──────────────────────────────────────────────────────────────
    test-results/tests-merge-commit-Merge-commit-tests-should-diff-file-after-clicking/video.webm
    ────────────────────────────────────────────────────────────────────────────────────────────────

  1 failed
    tests/merge-commit.spec.ts:59:7 › Merge commit tests › should diff file after clicking ─────────
  17 passed (1.8m)
error Command failed with exit code 1.

I would be glad to have your thoughts on this...

tsabbir96 · 2023-05-31T04:54:25Z

Hi @fcollonval sorry to bother you again. Could you provide some insights on this? I am stuck here and any feedback will be much appreciated.

fcollonval

Thanks @tsabbir96

Don't worry about the integration tests failing. They are not related to your work.

I only have small comments and this will be ready to go.

fcollonval · 2023-06-01T15:12:08Z

src/index.ts

@@ -185,6 +193,23 @@ async function activate(
    gitPlugin.title.icon = gitIcon;
    gitPlugin.title.caption = 'Git';

+    // Add the commands to the command palette
+    const category = 'Git Operations';


let's internationalize that string:

Suggested change

const category = 'Git Operations';

const category = trans.__('Git Operations');

fcollonval · 2023-06-01T15:13:32Z

src/index.ts

+      CommandIDs.gitOpenGitignore,
+      CommandIDs.gitShowDiff,
+      CommandIDs.gitInit,
+      CommandIDs.gitClone,


You must not add this one here as it is defined in another plugin.

Suggested change

CommandIDs.gitClone,

Instead you should add the same logic in the clone plugin there:

jupyterlab-git/src/cloneCommand.ts

Line 20 in dda8e82

export const gitCloneCommandPlugin: JupyterFrontEndPlugin<void> = {

You must not add this one here as it is defined in another plugin.

Instead you should add the same logic in the clone plugin there:

jupyterlab-git/src/cloneCommand.ts

Line 20 in dda8e82

export const gitCloneCommandPlugin: JupyterFrontEndPlugin<void> = {

@fcollonval Hi, thanks for the feedback. Could you kindly elaborate a bit for me on what you meant by "Instead you should add the same logic in the clone plugin there". Thank you once again.

gitCloneCommandPlugin is an extension plugin as the plugin you modified there:

jupyterlab-git/src/index.ts

Line 44 in fba3e2f

const plugin: JupyterFrontEndPlugin<IGitExtension> = {

So you should also add the ICommandPalette token as requirement and then call,

palette.addItem({ command: CommandIDs.gitClone, category: trans.__('Git Operations') })

welcome · 2023-08-08T13:17:55Z

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

tsabbir96 mentioned this pull request May 15, 2023

Feature Request: Git commands in the command palette #1238

Closed

fcollonval requested changes May 15, 2023

View reviewed changes

tsabbir96 changed the base branch from master to 0.9.x May 16, 2023 07:32

tsabbir96 changed the base branch from 0.9.x to master May 16, 2023 07:33

tsabbir96 requested a review from fcollonval May 18, 2023 05:59

fcollonval requested changes Jun 1, 2023

View reviewed changes

fcollonval force-pushed the add-git-to-command-palette branch from dc2cc2d to bb406b7 Compare August 3, 2023 15:51

tsabbir96 and others added 7 commits August 8, 2023 09:12

Add git to command palette

9044189

Add git to command palette

2181eb4

Add git to command palette

459f575

Add git to command palette

6f8d61f

Finish up the PR

e3e7ba5

Deduplicate yjs

af9f777

Give another shot at fixing yjs

ab6b108

fcollonval force-pushed the add-git-to-command-palette branch from 46a0707 to ab6b108 Compare August 8, 2023 07:12

fcollonval added 2 commits August 8, 2023 12:18

Fix tests

04a1a25

Lint the code

b31fb91

fcollonval approved these changes Aug 8, 2023

View reviewed changes

Use debug mode

80cc8b1

fcollonval added the enhancement label Aug 8, 2023

fcollonval added 2 commits August 8, 2023 14:54

Turn off debug

816cf29

Set up git identity for integration tests

6bbecdc

fcollonval merged commit c639273 into jupyterlab:main Aug 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add git to command palette #1243

Add git to command palette #1243

tsabbir96 commented May 15, 2023 •

edited

Loading

welcome bot commented May 15, 2023

github-actions bot commented May 15, 2023

fcollonval left a comment

fcollonval May 15, 2023

tsabbir96 May 15, 2023

fcollonval May 15, 2023

tsabbir96 commented May 16, 2023 •

edited

Loading

tsabbir96 commented May 31, 2023

fcollonval left a comment

fcollonval Jun 1, 2023

fcollonval Jun 1, 2023

tsabbir96 Jun 1, 2023

fcollonval Jun 2, 2023

welcome bot commented Aug 8, 2023

	const category = 'Git Operations';
	const category = trans.__('Git Operations');

Add git to command palette #1243

Add git to command palette #1243

Conversation

tsabbir96 commented May 15, 2023 • edited Loading

welcome bot commented May 15, 2023

github-actions bot commented May 15, 2023

fcollonval left a comment

Choose a reason for hiding this comment

fcollonval May 15, 2023

Choose a reason for hiding this comment

tsabbir96 May 15, 2023

Choose a reason for hiding this comment

fcollonval May 15, 2023

Choose a reason for hiding this comment

tsabbir96 commented May 16, 2023 • edited Loading

tsabbir96 commented May 31, 2023

fcollonval left a comment

Choose a reason for hiding this comment

fcollonval Jun 1, 2023

Choose a reason for hiding this comment

fcollonval Jun 1, 2023

Choose a reason for hiding this comment

tsabbir96 Jun 1, 2023

Choose a reason for hiding this comment

fcollonval Jun 2, 2023

Choose a reason for hiding this comment

welcome bot commented Aug 8, 2023

tsabbir96 commented May 15, 2023 •

edited

Loading

tsabbir96 commented May 16, 2023 •

edited

Loading