Re: [Mesa-dev] [PATCH] aubinator: Use less -RS instead of -r for the implicit pager.

[Gandikota, Sirisha]

>-Original Message-
>From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of
>Kenneth Graunke
>Sent: Monday, October 03, 2016 5:59 PM
>To: mesa-dev@lists.freedesktop.org
>Cc: Kenneth Graunke <kenn...@whitecape.org>
>Subject: [Mesa-dev] [PATCH] aubinator: Use less -RS instead of -r for the 
>implicit
>pager.
>
>From the less man page:
>
>   "Warning: when the -r option is used, less cannot keep track of the
>actual appearance  of  the screen (since this depends  on  how the
>screen responds to each type of control character).  Thus, various
>display problems may result, such as long lines being split in the
>wrong place."
>
>Lines which are too long to fit in the terminal would be word wrapped, but
>unfortunately less would get confused about which line it was on, and text 
>would
>be drawn on top of other text.  The most noticable case was shader assembly,
>which is frequently too wide for an 80 character terminal, and thus would be
>drawn on top of the following state packets, making them completely
>unreadable.
>
>Using -R instead of -r fixes this problem by only allowing color escape 
>sequences.
>(Notably, Git's implicit pager invocation uses -R.) Unfortunately, it means our
>"clear to the end of the line" hack for extending the blue bar headers won't 
>work
>anymore.
>
>Word wrapping usually isn't terribly readable, anyway, so we also add the -S
>option (chop long lines) to restrict it to the terminal width.
>(You can hit the left and right arrow keys to scroll sideways.)
>
>Then, for a new blue bar hack, we can use a printf specifier to pad the command
>packet names to be 80 characters long (arbitrarily), which extends them "far
>enough" to look good, and doesn't require us to use ioctls to determine the
>terminal width.
>
>Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
>---
> src/intel/tools/aubinator.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
>diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c index
>a31dcb2..5666fe3 100644
>--- a/src/intel/tools/aubinator.c
>+++ b/src/intel/tools/aubinator.c
>@@ -48,7 +48,6 @@
> #define CSI "\e["
> #define HEADER CSI "37;44m"
> #define NORMAL CSI "0m"
>-#define CLEAR_TO_EOL CSI "0K"
>
> /* options */
>
>@@ -721,7 +720,7 @@ parse_commands(struct gen_spec *spec, uint32_t
>*cmds, int size, int engine)
>   }
>   length = gen_group_get_length(inst, p);
>
>-  const char *color, *reset_color = CLEAR_TO_EOL NORMAL;
>+  const char *color, *reset_color = NORMAL;
>   uint64_t offset;
>
>   if (option_full_decode)
>@@ -739,7 +738,7 @@ parse_commands(struct gen_spec *spec, uint32_t
>*cmds, int size, int engine)
>   else
>  offset = 0;
>
>-  printf("%s0x%08lx:  0x%08x:  %s%s\n",
>+  printf("%s0x%08lx:  0x%08x:  %-80s%s\n",
>  color, offset, p[0],
>  gen_group_get_name(inst), reset_color);
>
>@@ -1013,7 +1012,7 @@ setup_pager(void)
>if (pid == 0) {
>   close(fds[1]);
>   dup2(fds[0], 0);
>-  execlp("less", "less", "-rFi", NULL);
>+  execlp("less", "less", "-FRSi", NULL);
>}
>
>close(fds[0]);
>--
>2.10.0
>
[SG]  Works for me
Reviewed-by: Sirisha Gandikota <sirisha.gandik...@intel.com>
>___
>mesa-dev mailing list
>mesa-dev@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] aubinator: Use less -RS instead of -r for the implicit pager.

[Kenneth Graunke]

From the less man page:

   "Warning: when the -r option is used, less cannot keep track of the
actual appearance  of  the screen (since this depends  on  how the
screen responds to each type of control character).  Thus, various
display problems may result, such as long lines being split in the
wrong place."

Lines which are too long to fit in the terminal would be word wrapped,
but unfortunately less would get confused about which line it was on,
and text would be drawn on top of other text.  The most noticable case
was shader assembly, which is frequently too wide for an 80 character
terminal, and thus would be drawn on top of the following state packets,
making them completely unreadable.

Using -R instead of -r fixes this problem by only allowing color escape
sequences.  (Notably, Git's implicit pager invocation uses -R.)
Unfortunately, it means our "clear to the end of the line" hack for
extending the blue bar headers won't work anymore.

Word wrapping usually isn't terribly readable, anyway, so we also add
the -S option (chop long lines) to restrict it to the terminal width.
(You can hit the left and right arrow keys to scroll sideways.)

Then, for a new blue bar hack, we can use a printf specifier to pad
the command packet names to be 80 characters long (arbitrarily), which
extends them "far enough" to look good, and doesn't require us to use
ioctls to determine the terminal width.

Signed-off-by: Kenneth Graunke 
---
 src/intel/tools/aubinator.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index a31dcb2..5666fe3 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -48,7 +48,6 @@
 #define CSI "\e["
 #define HEADER CSI "37;44m"
 #define NORMAL CSI "0m"
-#define CLEAR_TO_EOL CSI "0K"
 
 /* options */
 
@@ -721,7 +720,7 @@ parse_commands(struct gen_spec *spec, uint32_t *cmds, int 
size, int engine)
   }
   length = gen_group_get_length(inst, p);
 
-  const char *color, *reset_color = CLEAR_TO_EOL NORMAL;
+  const char *color, *reset_color = NORMAL;
   uint64_t offset;
 
   if (option_full_decode)
@@ -739,7 +738,7 @@ parse_commands(struct gen_spec *spec, uint32_t *cmds, int 
size, int engine)
   else
  offset = 0;
 
-  printf("%s0x%08lx:  0x%08x:  %s%s\n",
+  printf("%s0x%08lx:  0x%08x:  %-80s%s\n",
  color, offset, p[0],
  gen_group_get_name(inst), reset_color);
 
@@ -1013,7 +1012,7 @@ setup_pager(void)
if (pid == 0) {
   close(fds[1]);
   dup2(fds[0], 0);
-  execlp("less", "less", "-rFi", NULL);
+  execlp("less", "less", "-FRSi", NULL);
}
 
close(fds[0]);
-- 
2.10.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev