htop icon indicating copy to clipboard operation
htop copied to clipboard

Add F7 copy command functionality to CommandScreen

Open j-a-c-k-goes opened this issue 2 months ago • 3 comments

COPY_COMMAND_PR.md

Add F7 copy command functionality to CommandScreen

Implements copy-to-clipboard feature for process command lines as requested in issue #1788. Implementation uses F7 in CommandScreen to copy the full command line to system clipboard.

Features:

  • F7 key binding follows htop function key pattern
  • Linux/macOS/BSD via xclip/xsel/pbcopy support
  • Full command line copying w. all args
  • Operator feedback via status message
  • Error handling for missing clipboard tools

Implementation:

  • Added CommandScreen_copyCommand() function
  • Added CommandScreen_onKey() handler for F7
  • Updated function bar to show F7 Copy option
  • Platform-specific clipboard commands via system()
  • No external dependencies needed

F7 key binding addresses issue #1788 w/o breaking existing functionality. It is now convenient to copy process command lines for debugging, documentation, or reuse purposes.

Signed-off-by: [_ack] <[[email protected]]>

j-a-c-k-goes avatar Oct 24 '25 01:10 j-a-c-k-goes

  1. Please rebase the branch to remove the commit you made in #1792 (there is no dependency).
  2. The Signed-off-by line, as guided by various open-source projects, is supposed of include your real name. It indicates you either wrote this code entirely or you copied the code legally from someone else and have the legal right to contribute (the details is the Developer's Certificate of Origin). Different projects have different policies regarding the "Signed-off-by" requirement, but it's good to say that a "Signed-off-by" without a real name is useless.
  3. This patch introduced a dependency on external commands (not one, but two), there will be security implications to this. I think this feature is worth discussing further and never pull as is.

Explorer09 avatar Oct 24 '25 10:10 Explorer09

This PR has several issues, and one of them being down-voting of a good comment listing a few of the things that are wrong with this PR.

Let's start:

  1. The PR contains unrelated commits addressing an totally unrelated issue
  2. The PR contains new files implementing an test utility that is used nowhere
  3. That utility doesn't even follow the style guide (missing file header, missing LF @ EOF, the way if statements are one-liners, …)
  4. The Signed-off-by line that is missing a proper name (Certificate of Origin)

And given that htop regularly is running as root, there's quite an security impact with introducing those calls to external utilities. I know implementing this inside htop is just as ugly (after all you need to handle GDM, X11, Wayland, KDE, Gnome,DBus, …), but those system commands are an outright invitation for privilege escalation. Not to mention, that the exec itself should happen in a forked instance to avoid descriptor leakage.

BenBE avatar Oct 24 '25 21:10 BenBE

Another thing. No offense, but why does the test program test_copy_command.c contain emoji? It gives me an impression that @j-a-c-k-goes copied some code from an AI output. If that's the case, we have to reject the patches due to copyright concerns (and patches containing AI code would violate the Developer's Certificate of Origin they just signed).

Explorer09 avatar Oct 24 '25 22:10 Explorer09