Skip to content

unix: move clipboard utility discovery out of init()#64

Closed
tstromberg wants to merge 2 commits intoatotto:masterfrom
tstromberg:master
Closed

unix: move clipboard utility discovery out of init()#64
tstromberg wants to merge 2 commits intoatotto:masterfrom
tstromberg:master

Conversation

@tstromberg
Copy link
Copy Markdown

@tstromberg tstromberg commented May 18, 2022

Fixes #63

This moves the clipboard utility code from init() to a function named findClipboardUtility(). While this has a minor performance impact if the WriteAll or ReadAll command is called in a loop, this does improve binary start time.

It also looks a whole lot less scary in system call traces. This is particularly important for programs that indirectly import but never use this library.

As part of the cleanup, I removed references to an unused Primary global. Please let me know if you would like me to put them back.

@tstromberg tstromberg changed the title clipboard_unix: move utility discovery code out of init() unix: move utility discovery code out of init() May 18, 2022
@tstromberg tstromberg changed the title unix: move utility discovery code out of init() unix: move clipboard utility discovery code out of init() May 18, 2022
@tstromberg tstromberg changed the title unix: move clipboard utility discovery code out of init() unix: move clipboard utility discovery out of init() May 18, 2022

func readAll() (string, error) {
if Unsupported {
c := findClipboardUtility()
Copy link
Copy Markdown

@norbjd norbjd May 22, 2022

Choose a reason for hiding this comment

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

Hi 👋 just a small comment: instead of instantiating a new commandInfo every time readAll() or writeAll() is called, should it be possible to create a singleton:

 var instance commandInfo
 var once sync.Once

 func GetInstance() commandInfo {
     once.Do(func() {
         instance = findClipboardUtility()
     })
     return instance
 }

And call c := GetInstance() instead of c := findClipboardUtility()?

It will be quicker since findClipboardUtility() will be called only once (at the first call).

What do you think? BTW, thanks for this PR 🙌

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I considered this option, but it didn't seem worth the additional complication for a call that is unlikely to be called very often. If making this change helps the PR get merged though, I'm all for it.

dominikschulz added a commit to gopasspw/clipboard that referenced this pull request Apr 18, 2025
Inspired by atotto/clipboard#64.
Thanks @tstromberg and @norbjd.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
@tstromberg tstromberg closed this Jul 15, 2025
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.

Move scary $PATH clipboard utility scan out of init() to as-needed

2 participants