feat: copy to clipboard #234

0xbrayo · 2024-10-30T10:19:16Z

Closes #232

Important

Adds clipboard copy functionality with platform-specific handling, refactors utility functions, and integrates into shell and tmux modules.

Clipboard Functionality:
- Adds set_copytext() and copy() in clipboard.py for clipboard management.
- Supports Linux (xclip, wl-copy) and macOS (pbcopy).
Utility Functions:
- Moves get_system_distro() and get_installed_programs() to platform_utils.py and util.py.
- Refactors print_preview() in util.py to support clipboard copying.
Integration:
- Updates execute_shell() in shell.py and execute_tmux() in tmux.py to use print_preview() with clipboard copy option.

^{This description was created by}^{for 28e11a4. It will automatically update as commits are pushed.}

0xbrayo · 2024-10-30T10:45:35Z

Closes #232

0xbrayo · 2024-10-30T10:52:20Z

Experimented with a few approaches, ask_execute does not have access to the cmd variable. Passing it would involve rewriting all instances that confirmFunc is used. This was the simpler way I found to do it. Assumes that the print_preview is always called before ask_execute which is should always be the case. What do you think?

0xbrayo · 2024-10-30T11:54:01Z

Noticed that other confirmation dialogs such as overwrite file have the "copy" option.

ErikBjare

Looks good, nice workaround with the preview stuff, but some concerns about pyperclip.

pyproject.toml

ErikBjare · 2024-10-31T08:25:48Z

gptme/util.py

@@ -131,6 +133,7 @@ def epoch_to_age(epoch):
 def print_preview(code: str, lang: str):  # pragma: no cover
    print()
    print("[bold white]Preview[/bold white]")
+    set_text(code)


Won't really work for patches, but I guess that's ok for now.

poetry.lock

0xbrayo · 2024-10-31T08:54:08Z

~~I will try clipboard instead~~ Also not maintained. Will call the native copy commands directly.

gptme/tools/tmux.py

gptme/platform_utils.py

gptme/util.py

ellipsis-dev

❌ Changes requested. Reviewed everything up to f263c5d in 1 minute and 3 seconds

More details

Looked at 312 lines of code in 6 files
Skipped 0 files when reviewing.
Skipped posting 4 drafted comments based on config settings.

1. gptme/util.py:343

Draft comment:
The get_system_distro function is duplicated in multiple files. Consider defining it in one place and importing it where needed to avoid redundancy.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is speculative as it assumes the function is duplicated without providing evidence. The diff only shows the function being added here, and without evidence of duplication, the comment does not meet the criteria for being kept.
I might be missing the broader context of the codebase where this function could indeed be duplicated. However, based on the provided information, there's no evidence of duplication.
The task is to review the diff and comments based on the provided information. Without evidence of duplication in the diff or context, the comment should be removed.
Remove the comment as it is speculative and not based on evidence from the provided diff or context.

2. gptme/util.py:355

Draft comment:
The get_installed_programs function is duplicated in multiple files. Consider defining it in one place and importing it where needed to avoid redundancy.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is about a cross-file issue, which is not relevant to the changes made in this diff. The rules specify to ignore cross-file issues and focus only on the file being reviewed. Since there's no evidence of duplication within this file, the comment should be removed.
I might be missing the context of other files where this function could be duplicated, but the task is to focus only on the current file.
The rules are clear about ignoring cross-file issues, so the lack of evidence in this file is sufficient to disregard the comment.
Remove the comment as it pertains to a cross-file issue, which is not relevant to the current diff.

3. gptme/util.py:153

Draft comment:
The print_preview function has been modified to include a copy parameter, but the logic for setting copiable and set_copytext is not clear. Ensure these are only set when necessary.
Reason this comment was not posted:
Confidence changes required: 50%
The print_preview function has been modified to include a copy parameter, but the logic for setting copiable and set_copytext is not clear. It should be ensured that these are only set when necessary.

4. gptme/platform_utils.py:20

Draft comment:
Duplicate function get_installed_programs found in gptme/util.py. Use the existing function instead of adding a new one.
function get_installed_programs (util.py)
Reason this comment was not posted:
Marked as duplicate.

Workflow ID: wflow_n5fjgRTZQRD07l0Y

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

gptme/clipboard.py

feat: add copiable variable fix: update the copystr refactor: revert to "[c] to copy" feat: copy to clipboard refactor: revert to [c] to copy. handle circular import feat: simplify the input handling.

0xbrayo · 2024-11-08T11:16:43Z

@ellipsis-dev review

ellipsis-dev

❌ Changes requested. Reviewed everything up to 28e11a4 in 1 minute and 5 seconds

More details

Looked at 200 lines of code in 5 files
Skipped 0 files when reviewing.
Skipped posting 3 drafted comments based on config settings.

1. gptme/clipboard.py:28

Draft comment:
Consider using subprocess.run(..., check=True) to ensure that an exception is raised if the command fails, which can help in debugging and error handling.
Reason this comment was not posted:
Marked as duplicate.

2. gptme/clipboard.py:39

Draft comment:
Consider using subprocess.run(..., check=True) to ensure that an exception is raised if the command fails, which can help in debugging and error handling.
Reason this comment was not posted:
Marked as duplicate.

3. gptme/util.py:357

Draft comment:
The get_installed_programs function is duplicated in shell.py. Consider removing the one in shell.py to avoid redundancy.
Reason this comment was not posted:
Confidence changes required: 50%
The get_installed_programs function in util.py is duplicated in shell.py. The one in shell.py should be removed as it is not used and the one in util.py is more generic and used in clipboard.py.

Workflow ID: wflow_fskQ5vvxcb7pEBGE

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

gptme/clipboard.py

gptme/util.py

0xbrayo · 2024-11-09T11:23:23Z

@ErikBjare ping

ErikBjare

See the remaining ellipsis comments (should fix mypy + lint warnings), looks good otherwise. Haven't actually tested it.

gptme/util.py

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

ErikBjare · 2024-11-10T17:42:05Z

Nice, merged!

NikitaBeloglazov · 2024-12-16T23:02:56Z

Actually, I don't understand, there is the reason for building something unknown if you could just take clipman (an alternative to pyperclip) which was mentioned above. Or is there something missing?

ErikBjare · 2024-12-17T07:19:24Z

@NikitaBeloglazov There is no "building something unknown", the called executables should exist on their respective platforms.

I don't want to add another dependency for something so simple.

0xbrayo force-pushed the copy-to-clipboard branch from 9c9fdc1 to 876040b Compare October 30, 2024 10:42

0xbrayo requested a review from ErikBjare October 30, 2024 10:46

ErikBjare reviewed Oct 31, 2024

View reviewed changes

0xbrayo force-pushed the copy-to-clipboard branch 4 times, most recently from 7246af7 to e71aaa8 Compare November 1, 2024 07:27

ErikBjare reviewed Nov 1, 2024

View reviewed changes

gptme/tools/tmux.py Show resolved Hide resolved

gptme/platform_utils.py Outdated Show resolved Hide resolved

gptme/util.py Outdated Show resolved Hide resolved

gptme/util.py Outdated Show resolved Hide resolved

ErikBjare reviewed Nov 1, 2024

View reviewed changes

gptme/util.py Outdated Show resolved Hide resolved

0xbrayo force-pushed the copy-to-clipboard branch from a96b652 to f8cae58 Compare November 1, 2024 19:12

0xbrayo marked this pull request as ready for review November 7, 2024 13:08

ellipsis-dev bot reviewed Nov 7, 2024

View reviewed changes

gptme/clipboard.py Show resolved Hide resolved

0xbrayo force-pushed the copy-to-clipboard branch from 7c82ff4 to ec64640 Compare November 7, 2024 13:11

0xbrayo requested a review from ErikBjare November 7, 2024 13:12

feat: copy to clipboard

f1603fb

feat: add copiable variable fix: update the copystr refactor: revert to "[c] to copy" feat: copy to clipboard refactor: revert to [c] to copy. handle circular import feat: simplify the input handling.

0xbrayo force-pushed the copy-to-clipboard branch from 33693fc to 28e11a4 Compare November 8, 2024 11:15

ellipsis-dev bot reviewed Nov 8, 2024

View reviewed changes

gptme/clipboard.py Outdated Show resolved Hide resolved

gptme/util.py Show resolved Hide resolved

ErikBjare reviewed Nov 9, 2024

View reviewed changes

0xbrayo force-pushed the copy-to-clipboard branch from 40ecd1d to 6e4dfda Compare November 9, 2024 15:20

ErikBjare changed the title ~~feat: Copy to clipboard~~ feat: copy to clipboard Nov 9, 2024

ErikBjare reviewed Nov 9, 2024

View reviewed changes

gptme/util.py Outdated Show resolved Hide resolved

0xbrayo and others added 3 commits November 9, 2024 22:20

refactor: move get_installed_programs to utils

09070f1

feat: stop for copying only if override auto is not set

5f69980

Update gptme/clipboard.py

4e07159

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

0xbrayo force-pushed the copy-to-clipboard branch from 6e4dfda to 4e07159 Compare November 9, 2024 19:20

format: fixed formatting

b9f7701

ErikBjare merged commit 7ad4c94 into ErikBjare:master Nov 10, 2024
4 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: copy to clipboard #234

feat: copy to clipboard #234

0xbrayo commented Oct 30, 2024 •

edited by ErikBjare

Loading

0xbrayo commented Oct 30, 2024

0xbrayo commented Oct 30, 2024

0xbrayo commented Oct 30, 2024

ErikBjare left a comment

ErikBjare Oct 31, 2024

0xbrayo commented Oct 31, 2024 •

edited

Loading

ellipsis-dev bot left a comment

0xbrayo commented Nov 8, 2024

ellipsis-dev bot left a comment

0xbrayo commented Nov 9, 2024

ErikBjare left a comment

ErikBjare commented Nov 10, 2024

NikitaBeloglazov commented Dec 16, 2024 •

edited

Loading

ErikBjare commented Dec 17, 2024

feat: copy to clipboard #234

feat: copy to clipboard #234

Conversation

0xbrayo commented Oct 30, 2024 • edited by ErikBjare Loading

0xbrayo commented Oct 30, 2024

0xbrayo commented Oct 30, 2024

0xbrayo commented Oct 30, 2024

ErikBjare left a comment

Choose a reason for hiding this comment

ErikBjare Oct 31, 2024

Choose a reason for hiding this comment

0xbrayo commented Oct 31, 2024 • edited Loading

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

0xbrayo commented Nov 8, 2024

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

0xbrayo commented Nov 9, 2024

ErikBjare left a comment

Choose a reason for hiding this comment

ErikBjare commented Nov 10, 2024

NikitaBeloglazov commented Dec 16, 2024 • edited Loading

ErikBjare commented Dec 17, 2024

0xbrayo commented Oct 30, 2024 •

edited by ErikBjare

Loading

0xbrayo commented Oct 31, 2024 •

edited

Loading

NikitaBeloglazov commented Dec 16, 2024 •

edited

Loading