feat: add os.copyToClipboardCmd to allow for a custom command #1055

[redstreet]

• feat: add os.copyToClipboardCmd to allow for a custom command

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.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

[jesseduffield]

Looks good!

There's an existing integration test, pkg/integration/tests/patch_building/copy_patch_to_clipboard.go which we've disabled due to a lack of clipboard on CI. Would you be able to unskip that test and tweak it so that we define in SetupConfig for copyToClipboardCmd to be echo "{{text}}" > clipboard" and then we assert that a file shows up called clipboard in the files panel with the expected contents? Lemme know if that makes sense

[jesseduffield]

I see that text is quoted here but that we're also quoting it in the example command above. Does that still work?

[redstreet]

Good catch. The example worked fine because there are another pair of outer enclosing quotes, but was confusing. Simplified.

[redstreet]

Makes sense. I did what you said, and ran
go run cmd/integration_test/main.go cli --sandbox patch_building/copy_patch_to_clipboard. Manually doing a copy executes the configured command correctly, evidenced by:

bash -c "echo "0553aecdb695ed0e5d70100b1b2af769176786c4" > clipboard"

in the Command log. However, I can't figure out where the clipboard file is created. If I give it an absolute path (eg /tmp/clipboard), that works. I can see the directory with pwd. But if I write to a file (date > date.txt), no new file appears in the repo as I'd expect. What am I missing?

Second issue: quoting in the command above doesn't look right. That wouldn't work if {{text}} contained a space. What would you recommend?

[jesseduffield]

@redstreet hmm, I'm surprised that the file isn't showing up in the repo. Could you commit your test so I can take a closer look?

As for the quoting, the command log may be adding quotes for the sake of reading, but the resulting command may work fine.

[redstreet]

Sure, committed the test. I've been testing it manually using --sandbox, so please ignore the rest of the test, which is not ready yet.

[jesseduffield]

Ah, so the reason for the file not showing up is that we don't refresh the files view upon copying to clipboard (nor do we have any file watching functionality in place). So you'll need to focus the files panel and press 'r' to refresh it. Then you'll see the clipboard file :)

[redstreet]

Ah, that was silly of me to miss that! Will submit the full test soon. Thank you.

[redstreet]

Ready for review. Caveats:

• the test checks for the presence of a new file named clipboard, but does not check its contents (the SHA). The SHA can probably be obtained from the commit log, but I don't know how
• it would be nice to check for "Copied to clipboard" which appears on the screen on copy. I don't know how to do this either. t.ExpectToast(Contains("Copied to clipboard")) errors out

[redstreet]

Quick ping 🙂

[jesseduffield]

That toast erroring out is interesting: we recently made it so that we don't progress the test until lazygit is idle again, which would mean waiting until after the toast is no longer shown before asserting on it. So that would explain why it's failing. I'm happy to just leave that out of the test.

I've pushed some fixes. We're good to go!

[redstreet]

Great, and thanks for the excellent guidance on this PR!

[jesseduffield]

No worries @redstreet, thanks for making it