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

Linux: show newlines in commands as ? instead of space #1472

Merged
merged 1 commit into from May 11, 2024

Conversation

cgzones
Copy link
Member

@cgzones cgzones commented May 6, 2024

Since \n is used internally by htop to split command lines, replace \n with \r in command lines to not display them as space.

Merging the command string will not work, but newlines in commands should be rather the exception.

@BenBE BenBE added the enhancement Extension or improvement to existing feature label May 6, 2024
@BenBE BenBE added this to the 3.4.0 milestone May 6, 2024
@@ -1172,6 +1172,12 @@ static bool LinuxProcessTable_readCmdlineFile(Process* process, openat_arg_t pro

/* newline used as delimiter - when forming the mergedCommand, newline is
* converted to space by Process_makeCommandStr */
if (argChar == '\n') {
/* Set to some other non-pritnable character */
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo.

@Explorer09
Copy link
Contributor

Why choose \r in particular? Is this meant to be a graphical representation of the LF?
I wonder if U+240A is useful at the moment.

@BenBE
Copy link
Member

BenBE commented May 9, 2024

The problem with U+240A at this particular code location is, that the string is still in UTF-8 at this location and thus doing this replacement would require additional memory to be allocated.

@Explorer09
Copy link
Contributor

The problem with U+240A at this particular code location is, that the string is still in UTF-8 at this location and thus doing this replacement would require additional memory to be allocated.

I just wish this problem to have a long term solution. There should be ideally be a uniform way to sanitize and escape the characters from the /proc/<pid>/cmdline inputs.

@BenBE
Copy link
Member

BenBE commented May 9, 2024

We could allocate the cmdline buffer twice: Once for the original and a second copy for display with special characters replaced by their visual replacements. Though we'd need to introduce some markup in there to distinguish a normal U+240A in the command line vs. the replacement U+240A from displaying the LF. Any ideas? Opinions?

Since \n is used internally by htop to split command lines, replace \n
with \r in command lines to not display them as space.

Merging the command string will not work, but newlines in commands
should be rather the exception.
@cgzones
Copy link
Member Author

cgzones commented May 11, 2024

This fix is just so that newlines in commands are not displayed as spaces, but as unknown characters (in RichString_writeFrom(Wide|Ascii)()).

@cgzones cgzones merged commit ef07d6e into htop-dev:main May 11, 2024
17 checks passed
@cgzones cgzones deleted the newline branch May 11, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Extension or improvement to existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants