| Message ID | 1441537269-13899-1-git-send-email-felix.kaiser@fxkr.net |
|---|---|
| State | Rejected, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Sun, 6 Sep 2015 13:01:09 +0200, Felix Kaiser wrote:
> This improves the readability of the output.
Or makes the readability worse by requiring more scrolling, depending
on number of interfaces you have and your taste. Many users prefer the
current format.
Also, as sad as it is, there are scripts out there that parse ip output
and I have no doubts this would break them.
Jiri
Excerpts from Jiri Benc's message of 2015-09-07 14:46:09 +0200: > Or makes the readability worse by requiring more scrolling, depending > on number of interfaces you have and your taste. Of course it's slightly subjective, but I think it improves readability quite a bit *especially* on servers with many interfaces. Without the additional lines, you can obviously fit more on one screen, but I don't think it's much, and I think that once you have a screenful of text, adding some whitespace makes it much, much easier to focus on the individual sections. > Many users prefer the current format. I've met users who prefer ifconfigs format (and have literally told me that they find ips hard to read), and I've met users who use a shell alias to hack newlines in (which is the motivation for this patch), and everyone that I've shown side-by-side comparisons has been supportive. > Also, as sad as it is, there are scripts out there that parse ip output > and I have no doubts this would break them. I'm aware. My opinion is that if someone parses that, they deserve the opportunity to fix their code. At the very least they could have used -oneline! The default output format is clearly intended for humans; it seems like no attention at all has been given to parseability. And that's as it should be - since, because it requires the fewest keystrokes, that's the format that nearly all interactive users are going to see, and the output should be optimized for them. If absolute compatibility even for badly written scripts that misuse the tool had to be maintained then that'd mean that no improvements could ever be made, and that'd be sad. Felix -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 07 Sep 2015 17:38:48 +0200, Felix Kaiser wrote: > My opinion is that if someone parses that, they deserve the opportunity > to fix their code. At the very least they could have used -oneline! No doubt. I also don't like scripts that parse the default "ip a" output. I just pointed out that there are different opinions and that it can have some consequences. Personally, I have no strong feeling against nor for the patch. It's up to Stephen to decide. > I've met users who use a shell alias to hack newlines in Which is not a bad solution, actually. Jiri
On Mon, 7 Sep 2015 17:54:11 +0200 Jiri Benc <jbenc@redhat.com> wrote: > On Mon, 07 Sep 2015 17:38:48 +0200, Felix Kaiser wrote: > > My opinion is that if someone parses that, they deserve the opportunity > > to fix their code. At the very least they could have used -oneline! > > No doubt. I also don't like scripts that parse the default "ip a" > output. > > I just pointed out that there are different opinions and that it can > have some consequences. Personally, I have no strong feeling against > nor for the patch. It's up to Stephen to decide. > > > I've met users who use a shell alias to hack newlines in > > Which is not a bad solution, actually. > > Jiri > Agreed with Jiri. I prefer the new shorter brief format, not longer format. Did you ever try a system with 10 interfaces or 1000 tunnels?? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Excerpts from Stephen Hemminger's message of 2015-09-07 11:09:32 -0700: > Agreed with Jiri. I prefer the new shorter brief format, not longer format. As long as it requires extra typing, few are going to use it. It's about the default. Did you try the brief format with a few v6 addresses on an interface? When the line gets wrapped, it gets hard to read. Besides, this isn't about the brief format, but about making the long format less unreadable. > Did you ever try a system with 10 interfaces or 1000 tunnels?? The largest I tried it on had 11 interfaces (8 of which are for tunnels). That's not 1000, but how many people have that? After all the positive feedback I got locally I'm genuinely surprised that you don't like it, but I guess that's the way it is. Thanks for looking at it, though! Felix -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/ip/ip_common.h b/ip/ip_common.h index f74face..c39601d 100644 --- a/ip/ip_common.h +++ b/ip/ip_common.h @@ -1,7 +1,8 @@ extern int get_operstate(const char *name); extern int print_linkinfo(const struct sockaddr_nl *who, struct nlmsghdr *n, - void *arg); + void *arg, + int nitem); extern int print_linkinfo_brief(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg); diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 2aa5fbf..8b696fd 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -682,7 +682,7 @@ int print_linkinfo_brief(const struct sockaddr_nl *who, } int print_linkinfo(const struct sockaddr_nl *who, - struct nlmsghdr *n, void *arg) + struct nlmsghdr *n, void *arg, int nitem) { FILE *fp = (FILE*)arg; struct ifinfomsg *ifi = NLMSG_DATA(n); @@ -736,6 +736,10 @@ int print_linkinfo(const struct sockaddr_nl *who, } } + if (nitem > 0) { + fprintf(fp, "\n"); // empty line + } + if (n->nlmsg_type == RTM_DELLINK) fprintf(fp, "Deleted "); @@ -1638,6 +1642,7 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action) ipaddr_filter(&linfo, &ainfo); } + int nitem = 0; for (l = linfo.head; l; l = l->next) { int res = 0; struct ifinfomsg *ifi = NLMSG_DATA(&l->h); @@ -1649,12 +1654,14 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action) ainfo.head, stdout); } else if (no_link || - (res = print_linkinfo(NULL, &l->h, stdout)) >= 0) { + (res = print_linkinfo(NULL, &l->h, stdout, nitem)) >= 0) { if (filter.family != AF_PACKET) print_selected_addrinfo(ifi, ainfo.head, stdout); if (res > 0 && !do_link && show_stats) print_link_stats(stdout, &l->h); + if (res > 0) + nitem++; } } fflush(stdout); diff --git a/ip/iplink.c b/ip/iplink.c index 97f46cd..da4697c 100644 --- a/ip/iplink.c +++ b/ip/iplink.c @@ -847,7 +847,7 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask) if (brief) print_linkinfo_brief(NULL, &answer.n, stdout); else - print_linkinfo(NULL, &answer.n, stdout); + print_linkinfo(NULL, &answer.n, stdout, 0); return 0; } diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c index 8bcf882..c95568e 100644 --- a/ip/ipmonitor.c +++ b/ip/ipmonitor.c @@ -87,7 +87,7 @@ static int accept_msg(const struct sockaddr_nl *who, if (n->nlmsg_type == RTM_NEWLINK || n->nlmsg_type == RTM_DELLINK) { ll_remember_index(who, n, NULL); print_headers(fp, "[LINK]", ctrl); - print_linkinfo(who, n, arg); + print_linkinfo(who, n, arg, 0); return 0; } if (n->nlmsg_type == RTM_NEWADDR || n->nlmsg_type == RTM_DELADDR) {
This improves the readability of the output. Signed-off-by: Felix Kaiser <felix.kaiser@fxkr.net> --- ip/ip_common.h | 3 ++- ip/ipaddress.c | 11 +++++++++-- ip/iplink.c | 2 +- ip/ipmonitor.c | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-)