fix copy to the linux system clipboard #120

Distantoff · 2024-10-18T10:06:37Z

Hello! Thank you for your project, I liked it, but immediately I came across that the clipboard of the system was not working. I found the reason why this happens - the instance of arboard clipboard must run throughout the lifetime of the entire program..
Instead of copying text, signals are sent in the vim module, which are processed in the app, as well as components.

achristmascarl

thank you for the contribution @Distantoff, and for the research into the cause!

i left one small piece of feedback, but other than that everything else looks good, and will merge it after doing some testing

src/components/editor.rs

…pboard

achristmascarl · 2024-10-19T05:40:51Z

(you can ignore this bot's feedback for now, it's just to flag anything else i might want to look at)

@greptileai

greptile-apps

PR Summary

This pull request addresses the Linux system clipboard functionality issue in the rainfrog project. The changes ensure that the arboard clipboard instance persists throughout the application's lifecycle.

Implemented CopyData action in src/app.rs for centralized clipboard handling
Modified src/components/editor.rs to use action-based clipboard operations
Refactored src/vim.rs to send clipboard actions instead of direct clipboard access

Security implications: The changes don't introduce new security risks, as they primarily modify internal clipboard handling.

Performance: The modifications should have minimal impact on performance, with a slight increase in memory usage due to the persistent clipboard instance.

_{3 file(s) reviewed, 8 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

src/app.rs

src/components/editor.rs

greptile-apps · 2024-10-19T05:41:21Z

src/components/editor.rs

@@ -115,6 +117,7 @@ impl<'a> Editor<'a> {

 impl<'a, DB: Database + DatabaseQueries> Component<DB> for Editor<'a> {
  fn register_action_handler(&mut self, tx: UnboundedSender<Action>) -> Result<()> {
+    self.vim_state.register_action_handler(self.command_tx.clone())?;


logic: self.command_tx is None here, so this call will always fail

src/vim.rs

greptile-apps · 2024-10-19T05:41:35Z

src/vim.rs

  pub fn with_pending(self, pending: Input) -> Self {
-    Self { mode: self.mode, pending }
+    Self { mode: self.mode, pending, command_tx: None }
+  }


logic: command_tx is set to None here, potentially losing the original transmitter

src/vim.rs

achristmascarl · 2024-10-19T06:08:14Z

hm after taking another look at vim_state and the command_tx registration, i think it might be better to always initialize vim_state with a pointer to a function in editor.rs that does the message sending instead since it already has the command_tx initialized.

i can make the edits directly after doing some testing if you dont mind

Distantoff · 2024-10-19T06:59:28Z

hm after taking another look at vim_state and the command_tx registration, i think it might be better to always initialize vim_state with a pointer to a function in editor.rs that does the message sending instead since it already has the command_tx initialized.

i can make the edits directly after doing some testing if you dont mind

editor.rs

Yes of course! Perhaps this is the best solution

achristmascarl · 2024-10-19T22:32:16Z

i couldn't get it to work with giving the vim_state a function pointer or closure to a function in editor, so i'll stick with your approach of having the command_tx registered in vim_state :)

did some profiling and cloning the command_tx every transition doesnt have any performance impact

Distantoff · 2024-10-20T01:51:45Z

i couldn't get it to work with giving the vim_state a function pointer or closure to a function in editor, so i'll stick with your approach of having the command_tx registered in vim_state :)

did some profiling and cloning the command_tx every transition doesnt have any performance impact

Yes, cloning command_tx as far as I know just copies the reference, so the number of sender can be created a lot

achristmascarl

closes #83 (hopefully)

github-actions · 2024-10-20T05:19:33Z

Included in release v0.2.8

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [achristmascarl/rainfrog](https://github.com/achristmascarl/rainfrog) | patch | `v0.2.7` -> `v0.2.8` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>achristmascarl/rainfrog (achristmascarl/rainfrog)</summary> ### [`v0.2.8`](https://github.com/achristmascarl/rainfrog/releases/tag/v0.2.8) [Compare Source](achristmascarl/rainfrog@v0.2.7...v0.2.8)  #### What's Changed - fix copy to the linux system clipboard by [@Distantoff](https://github.com/Distantoff) in achristmascarl/rainfrog#120 #### New Contributors - [@Distantoff](https://github.com/Distantoff) made their first contribution in achristmascarl/rainfrog#120 **Full Changelog**: achristmascarl/rainfrog@v0.2.7...v0.2.8 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Distantoff added 3 commits October 17, 2024 16:32

Fixed the problem the text from vim was not copied in os clipboard

bc3e27a

Remove CopyData action handle, because it's handled in app mod

542dd96

cargo fmt fixes

0c33b01

achristmascarl requested changes Oct 18, 2024

View reviewed changes

src/components/editor.rs Show resolved Hide resolved

Distantoff added 2 commits October 19, 2024 11:38

return CopyData action in editor, and add check to termux arboard cli…

e5bf9fa

…pboard

remove duplicate code

27bbedc

greptile-apps bot reviewed Oct 19, 2024

View reviewed changes

fix clippy

1fb0dfc

achristmascarl approved these changes Oct 20, 2024

View reviewed changes

achristmascarl linked an issue Oct 20, 2024 that may be closed by this pull request

fix X11 clipboard timeout on linux #83

Closed

achristmascarl merged commit 6ee8551 into achristmascarl:main Oct 20, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix copy to the linux system clipboard #120

fix copy to the linux system clipboard #120

Distantoff commented Oct 18, 2024 •

edited

Loading

achristmascarl left a comment

achristmascarl commented Oct 19, 2024

greptile-apps bot left a comment

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

achristmascarl commented Oct 19, 2024

Distantoff commented Oct 19, 2024

achristmascarl commented Oct 19, 2024

Distantoff commented Oct 20, 2024

achristmascarl left a comment

github-actions bot commented Oct 20, 2024

fix copy to the linux system clipboard #120

fix copy to the linux system clipboard #120

Conversation

Distantoff commented Oct 18, 2024 • edited Loading

achristmascarl left a comment

Choose a reason for hiding this comment

achristmascarl commented Oct 19, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

achristmascarl commented Oct 19, 2024

Distantoff commented Oct 19, 2024

achristmascarl commented Oct 19, 2024

Distantoff commented Oct 20, 2024

achristmascarl left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 20, 2024

Distantoff commented Oct 18, 2024 •

edited

Loading