lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Branch name string escaped twice when creating pull request

Open Bryaneven opened this issue 2 years ago • 10 comments

Describe the bug Branch name gets double escaped when creating a pull request from within Lazygit

To Reproduce

  1. Create a branch with / in its name
  2. Open a pull request
  3. Check the url in the browser tab : / is now %252F instead of %2F

Expected behavior To escape the branch name only once

Version info: Lazygit : commit=, build date=, build source=homebrew, version=0.40.2, os=darwin, arch=arm64, git version=2.38.1 Git: 2.38.1

Bryaneven avatar Oct 04 '23 13:10 Bryaneven

Having the same issue, and I want to note that this problem didn't occur before and is about 1-2 months old.

pprotas avatar Oct 12 '23 11:10 pprotas

Do you have any specific public examples I can reproduce it with?

Because I tried it on v0.40.2 (and latest master) with the branch fix-open-PR/double/escape, I press o on it and I get a "normal" URL - https://github.com/jesseduffield/lazygit/compare/fix-open-PR%2Fdouble%2Fescape?expand=1

The example in #3064 used gitlab, and I don't feel like creating an account there if I don't have to.

mark2185 avatar Oct 22 '23 05:10 mark2185

having the same issue and I'm also using gitlab. seems like its adding %252F instead of %2F

Lazygit: commit=5e388e21c8ca6aa883dbcbe45c47f6fdd5116815, build date=2023-08-07T14:05:48Z, build source=binaryRelease, version=0.40.2, os=darwin, arch=arm64, git version=2.39.3 (Apple Git-145) git: version 2.39.3 (Apple Git-145)

alex-splyt avatar Oct 24 '23 05:10 alex-splyt

I was wandering whether this is something that happens when the request gets handled by the OS-specific layers. The unit test cases seem to cover this specific scenario. See https://github.com/jesseduffield/lazygit/blob/1d1b8cc01f87bb3495426ac8d81d97573f6840d4/pkg/commands/hosting_service/hosting_service_test.go#L130

So when copying the URL into the clipboard the URL is correctly escaped. The URL gets escaped once again when fed into macOS open command. Thankfully there is a config option that might be suitable to replace %2F with '/' and thus avoiding. I haven't found any option to open command in MacOS that would allow us to control the behaviour.

Summary - it is not Lazygit that handles the escaping process incorrectly. And LG provides a config option that allows us users to configure it. Thus I decided to go for 'writing a shell script' reversing the escaped string before feeding it into Google Chrome.

smangels avatar Oct 27 '23 13:10 smangels

I've managed to solve it through configuration.

os:
  openLink: 'bash -C ${HOME}/.config/lazygit/scripts/chrome_open_url.sh {{link}}'
#!/usr/bin/env bash
#
# when calling open, provided by OS, any escape character
# is escaped again
# in case of Lazygit, we replace the %2F character with '/'
# and let 'open' command do it's job escaping any character
# that could be problematic when opened in a web browser
URL="$1"
if [[ -z ${URL} ]]; then
  echo "no URL given"
  exit 1
fi

CLEAN_URL=$(echo "${URL}" | sed -e 's/%2F/\//g')

if ! open "${CLEAN_URL}"; then
  echo "failed to open >${URL}<"
  exit 1
fi
exit 0

smangels avatar Oct 28 '23 07:10 smangels

@smangels this issue contains a much easier workaround, at the bottom of the first post: https://github.com/jesseduffield/lazygit/issues/3064

pprotas avatar Oct 28 '23 12:10 pprotas

Sounds great. I'll change my config accordingly. One file less... BR,

smangels avatar Oct 28 '23 13:10 smangels

I can conform that the workaround works as expected.

smangels avatar Oct 28 '23 20:10 smangels

Hi, the workaround doesn't work when there are other escaped characters in the URL. For me, it did not work when the branch name was something like feature/name#TSK-1234. I've tried to add more things to the regex for sed, but it did not work for # as it has to be escaped to work properly.

What I've noticed is that the open command on macOS only escapes the URL when there are other unescaped characters. If there aren't any, it does not escape it on its own.

I mean:

  • open "https://example.com?foo[bar]=feature%2Fname%23TSK-1234" opens https://example.com/?foo%5Bbar%5D=feature%252Fname%2523TSK-1234 in the browser (the brackets are escaped, but so is the percent sign).
  • But open "https://example.com?foo%5Bbar%5D=feature%2Fname%23TSK-1234" opens https://example.com/?foo%5Bbar%5D=feature%2Fname%23TSK-1234 (nothing is changed).

I changed the workaround to basically escape the brackets rather than unescape the slash:

os:
  openLink: open "$(echo "{{link}}" | sed 's/\[/\%5B/g; s/\]/\%5D/g')"

So, I believe it can be fixed in lazygit by escaping other parts of the pull request URL, not just the branch name.

karmelak avatar Jan 05 '24 16:01 karmelak

Something changed for me, and now I had to change the open command from https://github.com/jesseduffield/lazygit/issues/3052#issuecomment-1878921995 to this:

diff --git a/.config/lazygit/config.yml b/.config/lazygit/config.yml
index 9e8eb5e..426aa4b 100644
--- a/.config/lazygit/config.yml
+++ b/.config/lazygit/config.yml
@@ -68,7 +68,7 @@ os:
 
   # Create pull request command opens a URL with incorrect escaping #3064
   # https://github.com/jesseduffield/lazygit/issues/3064
-  openLink: open "$(echo "{{link}}" | sed 's/%2F/\//g')"
+  openLink: open -n "$(echo {{link}} | sed 's/%2F/\//g')"

Hope this helps someone with the same issue.

mikavilpas avatar Mar 05 '24 16:03 mikavilpas