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

Adding bindings to yank the current URL and the selected URL #225

Merged
merged 5 commits into from Apr 20, 2021

Conversation

lostleonardo
Copy link
Contributor

Many thanks for the excellent support regarding where to make the changes - and for developing a code base that allowed me to get something working with so little difficulty. :-)

I used the following clipboard library https://pkg.go.dev/github.com/atotto/clipboard?utm_source=godoc ... I don't think it needs cgo, although it does depend on xsel or xclip (and their equivalents on Windows and Mac OS). Hopefully that fits with what you were thinking.

Issue #220

…ding to yank the currently selected target URL.
@singalhimanshu
Copy link
Contributor

singalhimanshu commented Apr 13, 2021

I guess you should add bind_yank_page_uri and bind_yank_target_uri under https://github.com/makeworld-the-better-one/amfora/blob/fe35f45e8c337c8feb6fb21fd0abf7a744995dc8/config/default.go#L160
Otherwise ~/.config/amfora/config.toml won't have these options in default config.

@lostleonardo
Copy link
Contributor Author

Thanks @singalhimanshu. Now added.

Copy link
Owner

@makew0rld makew0rld left a comment

Choose a reason for hiding this comment

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

Thanks for creating this PR! See my comments below.

Sorry I didn't mention this in #220, but I using URL is better than URI, and that using the word copy is better than yank. I would recommend these two names in the config:

bind_copy_page_url
bind_copy_highlight_url

Get rid of the addToClipboard function, and instead call clipboard.WriteAll directly. Capture the error, and

display/tab.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
…yank' to 'copy', plus a bit of general tidy up.
Copy link
Owner

@makew0rld makew0rld left a comment

Choose a reason for hiding this comment

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

Thanks for updating! See my comments below.

display/display.go Outdated Show resolved Hide resolved
display/display.go Show resolved Hide resolved
@makew0rld
Copy link
Owner

Looks great! I tested and it all works fine. Thanks!

@makew0rld makew0rld merged commit 40e9d21 into makew0rld:master Apr 20, 2021
makew0rld added a commit that referenced this pull request Apr 20, 2021
@lostleonardo lostleonardo deleted the enhancement/yank-uri branch April 22, 2021 14:32
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.

None yet

3 participants