Skip to content

Conversation

@j-a-c-k-goes
Copy link

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]]>

xyz32 added 2 commits October 23, 2025 17:16
Consolidates vector sorting functions as mentioned in issue htop-dev#1785.
Implements Highlander Rule by providing single sorting function.

Features:
- Context-aware comparison functions for sorting logic
- Hybrid algo: insertion sort <=16 elements, quicksort >16 elements
- Backward compatible. Existing functions unchanged
- NULL-safe context handling

Performance benefits:
- Algo selection based on array sizing
- Faster inserts on large arrays than w/ pure insertion sort
- Standard 16 element thresh for algo switching

This address issue htop-dev#1785 w/o breaking existing code.
Future commits can migrate call sites to use Vector_sort gradually (if merged)
Implements copy-to-clipboard feature for process command lines
as requested in issue htop-dev#1788. Implementation use 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 htop-dev#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]]>
@Explorer09
Copy link
Contributor

  1. Please rebase the branch to remove the commit you made in perf-enhancement: Add Vector_sort function w/ context support #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.

@BenBE
Copy link
Member

BenBE commented Oct 24, 2025

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 BenBE linked an issue Oct 24, 2025 that may be closed by this pull request
@Explorer09
Copy link
Contributor

Explorer09 commented Oct 24, 2025

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).

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.

Feature request - copy the command line

3 participants