iproute2mac icon indicating copy to clipboard operation
iproute2mac copied to clipboard

feat: Implement color output

Open luoling8192 opened this issue 1 year ago • 1 comments

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

luoling8192 avatar Nov 16 '24 18:11 luoling8192

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

brona avatar Nov 24 '24 09:11 brona

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

LordShedy avatar Apr 11 '25 20:04 LordShedy

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.

brona avatar Apr 16 '25 19:04 brona

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 avatar Apr 18 '25 17:04 luoling8192

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

luoling8192 avatar Apr 30 '25 11:04 luoling8192

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

alifiroozi80 avatar Jul 17 '25 14:07 alifiroozi80

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 avatar Jul 29 '25 19:07 brona