troglobit / pimd

PIM-SM/SSM multicast routing for UNIX and Linux
http://troglobit.com/projects/pimd/
BSD 3-Clause "New" or "Revised" License
194 stars 86 forks source link

Some report layout fixes #164

Closed LouisAtGH closed 3 years ago

LouisAtGH commented 3 years ago

When implementing a gui-package for pfSense, I noticed a couple of issues related to "pmctl --plain SHOW. I tried to improve a couple of things:

pimctl has more report options. I assume other reports will profit from the changes in pimctl.c as well, however I did not test that.

I noted that the report is showing a RP-address related to IPV4-link local !?

LouisAtGH commented 3 years ago

Here two pictures to show how the pimctl --plain show looks in pfSense

20200827 OutputPimctl_1 20200827 OutputPimctl_2

LouisAtGH commented 3 years ago

I just adding some pictures, never had the intention to close something :)

troglobit commented 3 years ago

Hi Louis, sorry for the late reply. I'm slowly getting back to working on the PIM projects again.

I think I understand what you're trying to do, and you're right. The --plain output was a bit broken, lacking the table headers. I'll see what I can do to clean up your commits so you get credit in the git logs.

Hopefully tomorrow, or later this week unless work takes all my time.


On an unrelated note, for better output in pfSense, I'd suggest adding some JavaScript support for parsing the ANSI control characters instead of using --plain. We've done so while integrating a few command line tools in Web interfaces in a few projects at work and the result is really great. Probably outside the scope of your work with pimd and pfSense, but maybe something to suggest to the maintainers of pfSense?

troglobit commented 3 years ago

I ended up reducing this pull request to fixing the missing heading while calling pimctl with --plain. All the other changes I cannot accept since they do not line up with either; a) the coding style of the project, b) irrelevant to the project (.dirstamp is not a file that should be part of GIT), or c) not in line with the overall design for the other multicast routing daemons (mrouted, pim6sd, pimd-dense).

LouisAtGH commented 3 years ago

Tja ...... I do not think I agree ...... IMHO I did improve the layout ..... little, if any thing left, from that ...... I will have a look at the end result .... of course ...

troglobit commented 3 years ago

The word "improve" is very subjective, we have different opinions and different end goals. Mine is to ensure all my FOUR (4!) different multicast routing daemons work and behave similar across many different operating systems. I hope you can understand and appreciate the work I've put into ensuring that. I stated the reasons plainly, but I could have just closed your pull-request without as much as a word. Instead, I spent an hour reading, testing, cleaning up, and trying to understand your change.