Skip to content

feat: Implement color output#63

Closed
luoling8192 wants to merge 2 commits intobrona:masterfrom
luoling8192:master
Closed

feat: Implement color output#63
luoling8192 wants to merge 2 commits intobrona:masterfrom
luoling8192:master

Conversation

@luoling8192
Copy link
Contributor

CleanShot 2024-11-17 at 02 49 11@2x

@brona
Copy link
Owner

brona commented Nov 24, 2024

Thank you for the PR, I will review it next week.

@LordShedy
Copy link

Any news about implementing/merging this feature? As of 1.5.4 it is not implemented.

@brona
Copy link
Owner

brona commented Apr 16, 2025

Unfortunately, I didn't get back to this. As this is adding considerable amount of code and a class...

Anyway two things that are possibly missing here:

  1. Similar support for the bridge command which is in separate file
  2. Perhaps a bit of comparison between the implementation / what is in the linux version, to make sure it is identical, as I don't see that in the tests or elsewhere - probably should be discussed at least here.

Regarding the class added to iproute2mac.py file, I wonder if you could turn that into functions / or refactor that out into a separate file.

@luoling8192
Copy link
Contributor Author

I have updated the new color class implementation in a separate file and completed the bridge color output. It works on my machine, so it should work for you as well.

CleanShot 2025-04-19 at 01 29 45@2x

@luoling8192 luoling8192 mentioned this pull request Apr 18, 2025
@luoling8192
Copy link
Contributor Author

Any news about implementing/merging this feature? As of 1.5.4 it is not implemented.

I have a new fork for that, with ss command support.

https://github.com/luoling8192/iproute2mac-color

@alifiroozi80
Copy link

Hey @brona
Could you please complete your review and merge this? It has been open since last year, mate, come on!

brona added a commit that referenced this pull request Jul 29, 2025
- Initial implementation following ideas in #63, however changing the implementation to more closely match original behavior, adding original author to AUTHORS.
- Default mode is `never` as that is what is Debian at the moment
- Matching color scheme detection depending on `$COLORFGBG`
- `NO_COLOR` is respected even with `--color=auto` mode
-  Supportable terminals are detected via `tput colors` and matching strings against `$TERM`
@brona
Copy link
Owner

brona commented Jul 29, 2025

Thanks for the interest @alifiroozi80.

I finally got some time to look into this one more closely. However the author took some liberties with the implementation which I don't necessarily agree with.

Specifically:

  • The applied coloring didn't highlight the same elements as the Linux version (e.g. word inet instead of the address itself, etc.)
  • The auto mode was enabled by default, while in the library never is the default option
  • The coloring was not implemented across the other show commands for other modules.

That being said, big thanks @luoling8192 for starting this one! I took your ideas, merged them with mine and implemented that in 0b4bf6b, currently in HEAD, to recognize your contributions I added you to the AUTHORs file.

Any chance someone could test the version from HEAD to see if there are any issues to be addressed before release? Closing this PR as we can follow up on the linked issues.

@brona brona closed this Jul 29, 2025
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.

5 participants