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

Copy branch url to clipboard #3343

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

skolj
Copy link

@skolj skolj commented Feb 26, 2024

  • PR Description
    This change allows copying branch URL to clipboard in the branches view. The existing copy X items are moved to a Copy to Clipboard menu with the following copy options:
  • Branch name
  • Branch URL (key: u)
  • Pull request URL (key: p)

Fixes #1959

  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • Docs (specifically docs/Config.md) have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

Copy link
Collaborator

@mark2185 mark2185 left a comment

Choose a reason for hiding this comment

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

Looks good!

Have just one suggestion :)

pkg/gui/controllers/branches_controller.go Show resolved Hide resolved
@@ -1746,6 +1759,9 @@ func EnglishTranslationSet() TranslationSet {
CopyCommitURLToClipboard: "Copy commit URL to clipboard",
CopyCommitAuthorToClipboard: "Copy commit author to clipboard",
CopyCommitAttributeToClipboard: "Copy to clipboard",
CopyBranchAttributeToClipboard: "Copy to clipboard",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just thinking out loud: I'm okay with having two of the attributes with the same text, but if it happens for a third time maybe it's best to make them all the same attribute.

Copy link
Author

Choose a reason for hiding this comment

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

I think I'll just create a generic Copy to Clipboard attribute since the copy menu would have the same label everywhere.

Copy link
Author

Choose a reason for hiding this comment

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

looks like there's already a CopyToClipboard attribute.

@skolj skolj requested a review from mark2185 February 26, 2024 10:52
Copy link
Collaborator

@mark2185 mark2185 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I take it these are correct because I've only checked for github.

Copy link
Author

Choose a reason for hiding this comment

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

I couldn't verify the branch URL for bitbucket server. The others should be correct.

@mark2185 mark2185 added the enhancement New feature or request label Feb 26, 2024
@mark2185 mark2185 added the requires-maintainer-signoff PR is reviewed but requires final signoff label Mar 7, 2024
@karimkhaleel
Copy link
Contributor

karimkhaleel commented Mar 10, 2024

Getting some weird behaviors when in a repo with multiple remotes. But I think it's not due to the changes in this PR, but rather with the way getHostingServiceMgr is implemented.

Copy branch url seems to only use the base url of the upstream named origin. This results in a 404 if the upstream named origin does not have a branch with that name.

Maybe we can find a way to pass in the name of the upstream of the branch we want to get the url / open a pull request for? We can always default to origin.

Especially since this is already what we are doing in CheckRemoteBranchExists.

@skolj
Copy link
Author

skolj commented Mar 10, 2024

Valid point @karimkhaleel. I'm not sure how we'd deal with upstreams (for from and to branches) when creating pull requests given that lazygit has a pull request options menu where the target branch can be selected. The branch URL part is clear to me.

@karimkhaleel
Copy link
Contributor

I'm not sure how we'd deal with upstreams (for from and to branches) when creating pull requests given that lazygit has a pull request options menu where the target branch can be selected.

Maybe we just leave the to branch as default, like the first option in the View pull request options... menu? This is already what the lowercase o keybind does in the branches menu anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request requires-maintainer-signoff PR is reviewed but requires final signoff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy branch URL to clipboard
3 participants