add copy branch url to clipboard option in branches menu #2068
Conversation
|
I propose we put this and the other |
| func (self *BranchesController) copyBranchURL() error { | ||
| branch := self.context().GetSelected() | ||
|
|
||
| branchExistsOnRemote := self.git.Remote.CheckRemoteBranchExists(branch.Name) |
There was a problem hiding this comment.
This checks only for the hardcoded upstream of origin, look here:
// CheckRemoteBranchExists Returns remote branch
func (self *RemoteCommands) CheckRemoteBranchExists(branchName string) bool {
_, err := self.cmd.
New(
fmt.Sprintf("git show-ref --verify -- refs/remotes/origin/%s",
self.cmd.Quote(branchName),
),
).
DontLog().
RunWithOutput()
return err == nil
}
I propose we send the branch.UpstreamRemote as a parameter alongside branchName.
There was a problem hiding this comment.
Sorry, proposing that upstreamRemote gets passed in with the CheckRemoteBranchExists? Or with the getBranchURL method? Think I'm confused as to where that becomes helpful. The rest of the tweaks are finished, gonna wait for clarification on this one :)
There was a problem hiding this comment.
Sorry, proposing that upstreamRemote gets passed in with the CheckRemoteBranchExists?
It gets sent to CheckRemoteBranchExists.
Think I'm confused as to where that becomes helpful.
Try adding a new remote. It's not gonna be refs/remotes/origin/<something>, but it's gonna be e.g. refs/remotes/ryans-fork/<something>. But CheckRemoteBranchExists has origin hardcoded in its git command, how's that gonna work for something not on origin or if origin doesn't even exist? :)
| @@ -24,6 +25,7 @@ var bitbucketServiceDef = ServiceDefinition{ | |||
| pullRequestURLIntoDefaultBranch: "/pull-requests/new?source={{.From}}&t=1", | |||
| pullRequestURLIntoTargetBranch: "/pull-requests/new?source={{.From}}&dest={{.To}}&t=1", | |||
| commitURL: "/commits/{{.CommitSha}}", | |||
| branchURL: "/branch/{{.BranchName}}", | |||
There was a problem hiding this comment.
This isn't the same as github's tree view of a certain branch, but I'm not sure what would people prefer. I was expecting for it to open the view of the entire repo, but on a different branch.
This opens up a view of the actual branch, shows the most recent commits.
There is a button to take the user to view the source of the current branch, and the URL pattern looks like this:
/src/<SHA of HEAD>/?at=name-of-the-branch
It can be either the short SHA or the full one, makes no difference, unless there's a collision.
…h url functions to be used to properly generate bitbucket links
|
Life caught up with me this week, think I got everything here :) |
| } | ||
|
|
||
| return self.c.Menu(types.CreateMenuOptions{ | ||
| Title: fmt.Sprintf(self.c.Tr.CopyToClipboardMenuTitle), |
There was a problem hiding this comment.
| Title: fmt.Sprintf(self.c.Tr.CopyToClipboardMenuTitle), | |
| Title: self.c.Tr.CopyToClipboardMenuTitle, |
No need for the fmt.Sprintf when just rendering a single string
| } | ||
|
|
||
| // Get most recent commitSha to produce the proper bitbucket url | ||
| commitSha, err := self.git.Custom.RunWithOutput("git rev-parse --short HEAD") |
There was a problem hiding this comment.
git.Custom.RunWithOutput should only be used for when a user has entered the command themselves. Reason being that we want to have all our git-specific commands living in the git_commands package, so that if we ever decide to swap out the commands for actual git bindings (e.g. libgit2 or go-git) we don't need to touch any controller code.
I'd chuck a method called GetHeadCommitSha in pkg/commands/git_commands/commit.go which internally calls git rev-parse --short HEAD.
|
I get the |
| self.cmd.Quote(branchName), | ||
| self.cmd.Quote(upstreamRemote), |
There was a problem hiding this comment.
I think these are in the wrong order? First comes the upstreamRemote, then the branchName?
| @@ -502,7 +502,7 @@ func GetDefaultConfig() *UserConfig { | |||
| OpenStatusFilter: "<c-b>", | |||
| }, | |||
| Branches: KeybindingBranchesConfig{ | |||
| CopyPullRequestURL: "<c-y>", | |||
| CopyToClipboardMenu: "<c-y>", | |||
There was a problem hiding this comment.
Could we replace this <c-y> with just y, like in the Commits panel?
There was a problem hiding this comment.
When I had it as just y it would select the menu, and then just fall through to selecting the next option as well without letting us pick from the menu. Couldn't find why that was the case, maybe you have more insight?
There was a problem hiding this comment.
Could you check if the code is the same as for copying a commit attribute? It's in the Commits pane, also under y?
But first check if that works as expected on your machine.
There was a problem hiding this comment.
Definitely can, will be working on this today
There was a problem hiding this comment.
Looking into the commit pane impl, the y command for that is also just falling through to the first option. the menu never opens and it just selects the top option. the y key is also a universal binding for confirmAlt1 so that must be taking precedence. should i switch both of them to c-y? or find another way around it?
There was a problem hiding this comment.
Uhhh... happy debugging? Try it in a docker container, or with a clean config, and in an another terminal emulator.
There was a problem hiding this comment.
yuuppp, working on figuring it out now. very odd that it's not behaving for me.
There was a problem hiding this comment.
So experimenting with this, I've tried on two computers, in 3 different shells, and on my branch and in the current brew installation of the app and the master branch and y is consistently falling through to the first option for me. If it's working for you still then I've got no idea what's going on but I feel like the universal keybinding is taking precedence potentially 🤷♂️ But I'm not sure where else to go from here other than maybe mapping them both to c-y, or having them on y and trusting this is an isolated issue for my machines? lol
There was a problem hiding this comment.
rusting this is an isolated issue for my machines? lol
Something to be solved, then.
Have you tried:
- different keyboard layout (US seems to work for me)
- in
docker - different terminal emulator
Do the integration tests work okay? Can you replay them through the lazyintegration replay thingy?
There was a problem hiding this comment.
I've not tried a different layout, but y works for other commands like c-y so i don't think it's a layout issue but I'll see. I tried in stock MacOS terminal, fish, nushell, kitty, iterm, warp and alacritty (when i said shell above i meant terminal emulator, my bad) both on my desktop and a laptop that was wiped clean about 4 days ago so is basically stock, all on master branch, my personal branches, and a fresh brew install. I haven't tried docker yet, but could give it a shot tomorrow. I ran everything inside of lazyintegration and all of them passed fine. I'll give docker a shot and see, but I'd be lying if I said I wasn't doubtful
Before there was not an option to copy the branch URL to the clipboard. Now Ctrl + B will copy branch URL to keyboard.
I believe git service paths are correct, was gathered through a combination of my own accounts and watching branches tutorials on YouTube to see how the path changes :) (azure devops, bitbucket server specifically). Let me know if anything needs to be changed. Thanks!
Related Issue: #1959