Skip to content
This repository has been archived by the owner on Aug 29, 2020. It is now read-only.

Fix inability to display command names with whitespace in macOS #109

Merged
merged 2 commits into from
Feb 11, 2019

Conversation

whuang8
Copy link
Contributor

@whuang8 whuang8 commented Feb 9, 2019

Description

ps parsing in gotop is different for macOS and Linux because you can define column length on Linux but not on macOS. However, on macOS, the -o flag allows you to define custom headers. By setting a header to a specific length, we can recreate the Linux functionality of setting column widths.

header's width = column width

Now, parsing ps output can be the same as that defined in proc_linux.go

Fixes #107

@whuang8 whuang8 changed the title Fix inability to display command names with whitespace in MacOS Fix inability to display command names with whitespace in macOS Feb 9, 2019
@whuang8
Copy link
Contributor Author

whuang8 commented Feb 9, 2019

Definitely a bit of a hack to define column widths by the header 😅

Any thoughts?

@cjbassi
Copy link
Owner

cjbassi commented Feb 10, 2019

Nice! This looks good. It is a little hacky, but I'd rather call that ingenuitive 😄

So you've tested this on OSX and its working for you?

Also, there was an issue where the command name was being cropped on OSX: #47 That was solved by adding the -ww flags, so maybe we don't need those anymore? Can you make sure both the command names and the full process names aren't being cropped with these changes and then try removing -ww and seeing if things are still good? The full process names are visible by pressing <Tab>. Thanks!

@whuang8
Copy link
Contributor Author

whuang8 commented Feb 10, 2019

I removed the -ww flags and tested this change on my Mac. The command names look goo and I can now see Google Chrome as well as other command names with white space.

image

I see this after pressing tab

image

Some process names after pressing tab are a little too long to fit in the panel. For example, take a look at Google Chrome Helper

image

If we take a look at these names with a bigger screen, more information shows up.

image

Maybe we can recreate htop's functionality of horizontal scroll with the ← → keys some time in the future?

@cjbassi
Copy link
Owner

cjbassi commented Feb 11, 2019

Awesome, that looks good. Yah trimming the long process names is the expected behavior (for now) so that's fine, but maybe something to look into fixing in the future. Thanks a lot for the changes! Merging this and I'll try to create a release in a bit.

@cjbassi cjbassi merged commit 5259093 into cjbassi:master Feb 11, 2019
@whuang8 whuang8 deleted the bug/show-chrome-processes branch February 11, 2019 06:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants