[PATCH iproute2-next 0/3] bridge: fixes regarding the colorized output

[PATCH iproute2-next 0/3] bridge: fixes regarding the colorized output

[Gokul Sivakumar]

For the bridge cmd, the "colorize output" cmd line option's usage and the
corresponding output does not seem to be consistent with the usage and the
output of the "ip" cmds.

Example 1: As per the "colorize output" cmd line option documented in the
man pages man/man8/ip.8 & man/man8/bridge.8, using "-c" with "ip" or the
"bridge" should colorize the output. But this is working only with the "ip"
cmd and for the "bridge" cmd, the option "-col" / "-colo" / "-color" needs
to be used instead of just "-c".

Example 2: After fixing the inconsistency mentioned in Example 1,
- Running the cmd "$ ip -c neigh", will give the following output where the
  "172.16.12.250", "bridge0" & "04:d9:f5:c1:0c:74" fields are highlighted
  and not the fixed keywords "dev" & "lladdr".

  172.16.12.250 dev bridge0 lladdr 04:d9:f5:c1:0c:74 REACHABLE

- But running the cmd "$ bridge -c fdb", will give the following output
  where "00:00:00:00:00:00", "dev", "vxlan100", "dst" & "2001:db8:2::1" are
  highlighted, even though "dev" & "dst" keywords are static.

  00:00:00:00:00:00 dev vxlan100 dst 2001:db8:2::1 self permanent

Also fix a typo in man/man8/bridge.8 to change "-c[lor]" into "-c[olor]".

Gokul Sivakumar (3):
  bridge: reorder cmd line arg parsing to let "-c" detected as "color"
    option
  bridge: fdb: don't colorize the "dev" & "dst" keywords in "bridge -c
    fdb"
  man: bridge: fix the typo to change "-c[lor]" into "-c[olor]" in man
    page

 bridge/bridge.c   |  2 +-
 bridge/fdb.c      | 13 ++++++++++---
 man/man8/bridge.8 |  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH iproute2-next 1/3] bridge: reorder cmd line arg parsing to let "-c" detected as "color" option

[Gokul Sivakumar]

As per the man/man8/bridge.8 page, the shorthand cmd line arg "-c" can be
used to colorize the bridge cmd output. But while parsing the args in while
loop, matches() detects "-c" as "-compressedvlans" instead of "-color", so
fix this by doing the check for "-color" option first before checking for
"-compressedvlans".

Signed-off-by: Gokul Sivakumar <gokulkumar792@gmail.com>
---
 bridge/bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bridge/bridge.c b/bridge/bridge.c
index f7bfe0b5..48b0e7f8 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -149,9 +149,9 @@ main(int argc, char **argv)
 			NEXT_ARG();
 			if (netns_switch(argv[1]))
 				exit(-1);
+		} else if (matches_color(opt, &color)) {
 		} else if (matches(opt, "-compressvlans") == 0) {
 			++compress_vlans;
-		} else if (matches_color(opt, &color)) {
 		} else if (matches(opt, "-force") == 0) {
 			++force;
 		} else if (matches(opt, "-json") == 0) {
-- 
2.25.1

[PATCH iproute2-next 2/3] bridge: fdb: don't colorize the "dev" & "dst" keywords in "bridge -c fdb"

[Gokul Sivakumar]

To be consistent with the colorized output of "ip" command and to increase
readability, stop highlighting the "dev" & "dst" keywords in the colorized
output of "bridge -c fdb" cmd.

Example: in the following "bridge -c fdb" entry, only "00:00:00:00:00:00",
"vxlan100" and "2001:db8:2::1" fields should be highlighted in color.

00:00:00:00:00:00 dev vxlan100 dst 2001:db8:2::1 self permanent

Signed-off-by: Gokul Sivakumar <gokulkumar792@gmail.com>
---
 bridge/fdb.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 37465e46..21433dac 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -192,10 +192,14 @@ int print_fdb(struct nlmsghdr *n, void *arg)
 				   "mac", "%s ", lladdr);
 	}
 
-	if (!filter_index && r->ndm_ifindex)
+	if (!filter_index && r->ndm_ifindex) {
+		if (!is_json_context())
+			fprintf(fp, "dev ");
+
 		print_color_string(PRINT_ANY, COLOR_IFNAME,
-				   "ifname", "dev %s ",
+				   "ifname", "%s ",
 				   ll_index_to_name(r->ndm_ifindex));
+	}
 
 	if (tb[NDA_DST]) {
 		int family = AF_INET;
@@ -208,9 +212,12 @@ int print_fdb(struct nlmsghdr *n, void *arg)
 				  RTA_PAYLOAD(tb[NDA_DST]),
 				  RTA_DATA(tb[NDA_DST]));
 
+		if (!is_json_context())
+			fprintf(fp, "dst ");
+
 		print_color_string(PRINT_ANY,
 				   ifa_family_color(family),
-				    "dst", "dst %s ", dst);
+				    "dst", "%s ", dst);
 	}
 
 	if (vid)
-- 
2.25.1

[PATCH iproute2-next 3/3] man: bridge: fix the typo to change "-c[lor]" into "-c[olor]" in man page

[Gokul Sivakumar]

Fixes: 3a1ca9a5b ("bridge: update man page for new color and json changes")
Signed-off-by: Gokul Sivakumar <gokulkumar792@gmail.com>
---
 man/man8/bridge.8 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index eec7df43..db83a2a6 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -22,7 +22,7 @@ bridge \- show / manipulate bridge addresses and devices
 \fB\-s\fR[\fItatistics\fR] |
 \fB\-n\fR[\fIetns\fR] name |
 \fB\-b\fR[\fIatch\fR] filename |
-\fB\-c\fR[\folor\fR] |
+\fB\-c\fR[\fIolor\fR] |
 \fB\-p\fR[\fIretty\fR] |
 \fB\-j\fR[\fIson\fR] |
 \fB\-o\fR[\fIneline\fr] }
-- 
2.25.1

Re: [PATCH iproute2-next 2/3] bridge: fdb: don't colorize the "dev" & "dst" keywords in "bridge -c fdb"

[Stephen Hemminger]

On Sat, 14 Aug 2021 15:24:38 +0530
Gokul Sivakumar <gokulkumar792@gmail.com> wrote:

> +	if (!filter_index && r->ndm_ifindex) {
> +		if (!is_json_context())
> +			fprintf(fp, "dev ");

This looks functionally correct, but please use:
            print_string(PRINT_FP, NULL, "dev ", NULL);

The reason as part of the json conversions I look for fprintf(fp
as indicator of unconverted code.

Re: [PATCH iproute2-next 2/3] bridge: fdb: don't colorize the "dev" & "dst" keywords in "bridge -c fdb"

[Gokul Sivakumar]

On Sat, 2021-08-14 at 09:22 -0700, Stephen Hemminger wrote:
> On Sat, 14 Aug 2021 15:24:38 +0530
> Gokul Sivakumar <gokulkumar792@gmail.com> wrote:
> 
> > +	if (!filter_index && r->ndm_ifindex) {
> > +		if (!is_json_context())
> > +			fprintf(fp, "dev ");
> 
> This looks functionally correct, but please use:
>             print_string(PRINT_FP, NULL, "dev ", NULL);
> 
> The reason as part of the json conversions I look for fprintf(fp
> as indicator of unconverted code.

Ok, thanks for the review. I will send an updated v2 patchset now
with the 2 newly added fprintf() calls replaced by print_string().