Skip to content

Feat: add button at scm title#41

Closed
Chzxxuanzheng wants to merge 10 commits into
asispts:mainfrom
Chzxxuanzheng:dev
Closed

Feat: add button at scm title#41
Chzxxuanzheng wants to merge 10 commits into
asispts:mainfrom
Chzxxuanzheng:dev

Conversation

@Chzxxuanzheng

Copy link
Copy Markdown
Contributor
Q A
Type feature
Issue Add a button at SCM title
图片

I don't test whether it support git submodule, due to #23 don't merged

@Chzxxuanzheng Chzxxuanzheng requested a review from asispts as a code owner April 9, 2026 13:31
Copilot AI review requested due to automatic review settings April 9, 2026 13:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an SCM title-bar entry point to open the Git Graph webview and updates repo-selection behavior so invoking the command from SCM can switch the active repository shown in the webview.

Changes:

  • Contribute neo-git-graph.view to the scm/title menu (SCM title button) and add an icon for the command.
  • Allow neo-git-graph.view to accept a resource argument and attempt to switch the graph to that repository (including when reusing an existing panel).
  • Fix merge confirmation dialog placeholder substitution for branch merges by filling {1}.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/webview/main.ts Updates repo switching logic driven by lastActiveRepo; fixes merge confirm placeholder substitution for branch merges.
src/extension/webviewPanel.ts Adds initialRepo option to seed lastActiveRepo before webview HTML/state is built.
src/extension.ts Extends neo-git-graph.view to accept a resource argument and push repo changes to an existing webview via the bridge.
package.json Adds SCM title menu contribution and command icon metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/webview/main.ts
Comment thread src/webview/main.ts
Comment thread src/extension.ts
Comment thread src/extension.ts Outdated
Comment thread package.json
@Chzxxuanzheng

Copy link
Copy Markdown
Contributor Author

I think add a checkbox list like below at PR template is better for the developer who first contribute to this resp or who often forget to do this.

- [ ] have run `pnpm run test`
- [ ] have run `xvfb-run -a pnpm run test:ext`
- [ ] have run `pnpm run typecheck`
- [ ] have run `pnpm run lint:fix`
- [ ] have run `pnpm run format:fix`

Copilot AI review requested due to automatic review settings April 9, 2026 14:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/webview/main.ts
Comment thread src/extension.ts
Comment thread src/extension.ts
Comment on lines +48 to +52
} else if (typeof resource?.uri?.fsPath === "string") {
repoPath = resource.uri.fsPath;
}
}

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

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

repoPath is derived from resource.rootUri.fsPath / resource.uri.fsPath, but this codebase normalizes repo paths to forward slashes (see getPathFromUri/getPathFromStr). On Windows, using fsPath directly will leave backslashes and fail the repos[repoPath] lookup, so the SCM-title button won’t select the intended repo. Normalize the URI before comparing/using it (and consider the case where resource itself is a vscode.Uri).

Copilot uses AI. Check for mistakes.
Comment thread package.json
Comment on lines +80 to +100
"title": "%command.view%",
"icon": {
"light": "resources/webview-icon-light.svg",
"dark": "resources/webview-icon-dark.svg"
}
},
{
"category": "(neo) Git Graph",
"command": "neo-git-graph.clearAvatarCache",
"title": "%command.clearAvatarCache%"
}
],
"menus": {
"scm/title": [
{
"command": "neo-git-graph.view",
"group": "navigation",
"when": "scmProvider == git"
}
]
},

@asispts asispts Apr 9, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This code alone can do the job without modifying/adding any other code. The existing command handler can handle the active/current repo state by itself.

So, you can remove the other changes. Any future issues will be solved independently.

@Chzxxuanzheng Chzxxuanzheng Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Only adding this code will result in it only open the last repo when face multi repo. I think if without the auto switch repo function, the PR will give a scm title button with bug.

You mean I should put it split into two PR, or you will solve this questions yourself?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yeah, I think we can split the PR. You can propose the fix for the multi-repo issue. But be aware that I'm preparing to release a new version. After releasing it, I'll be working on the initialization logic that may affect this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants