diff mbox series

package/readline: disable bracketed paste by default

Message ID 20210226200821.2992417-1-mmayer@broadcom.com
State Accepted
Headers show
Series package/readline: disable bracketed paste by default | expand

Commit Message

Markus Mayer Feb. 26, 2021, 8:08 p.m. UTC 
As of readline 8.1, "bracketed paste" is enabled by default. However,
the feature causes control characters to appear in captured (telnet)
session output. This can throw off pattern matching if the output is to
be processed by scripts.

Let's keep the previous default of leaving this feature disabled and
provide a configuration option for users to enable it.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---

We started running into issues after bringing in the changes of the upcoming
BR 2021.02 release, because raw output with the feature enabled looks like
this, with interspersed escape sequences like "<ESC>.[?2004h":

< 0x00000: 1b 5b 3f 32  30 30 34 68  24 20                     .[?2004h$ 
> 0x00000: 70 73 20 78  0d 0a                                  ps x..
< 0x00000: 70 73 20 78                                         ps x
< 0x00000: 0d 0a 1b 5b  3f 32 30 30  34 6c 0d                  ...[?2004l.
< 0x00000: 20 20 50 49  44 20 54 54  59 20 20 20  20 20 20 53    PID TTY      S
< 0x00010: 54 41 54 20  20 20 54 49  4d 45 20 43  4f 4d 4d 41  TAT   TIME COMMA
< 0x00020: 4e 44 0d 0a                                         ND..
< 0x00000: 20 31 36 36  39 20 70 74  73 2f 30 20  20 20 20 53   1669 pts/0    S
< 0x00010: 73 20 20 20  20 20 30 3a  30 30 20 2d  73 68 0d 0a  s     0:00 -sh..
< 0x00000: 20 31 36 37  31 20 70 74  73 2f 30 20  20 20 20 52   1671 pts/0    R
< 0x00010: 2b 20 20 20  20 20 30 3a  30 30 20 70  73 20 78 0d  +     0:00 ps x.
< 0x00020: 0a                                                  .
< 0x00000: 1b 5b 3f 32  30 30 34 68                            .[?2004h
< 0x00000: 24 20                                               $ 

Our automated test setup didn't like that too much. And others might be
facing similar issues.

Besides, Buildroot-based systems are probably not used interactively all
that often, so a feature that helps with interactive sessions, but makes
life harder for automation, should probably remain disabled by default. At
the same time, it should be possible to turn it on if desired.

 package/readline/Config.in   | 14 ++++++++++++++
 package/readline/readline.mk |  4 ++++
 2 files changed, 18 insertions(+)

Comments

Yann E. MORIN Feb. 26, 2021, 9:43 p.m. UTC | #1 
Markus, All,

On 2021-02-26 12:08 -0800, Markus Mayer via buildroot spake thusly:
> As of readline 8.1, "bracketed paste" is enabled by default. However,
> the feature causes control characters to appear in captured (telnet)
> session output. This can throw off pattern matching if the output is to
> be processed by scripts.
> 
> Let's keep the previous default of leaving this feature disabled and
> provide a configuration option for users to enable it.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
> 
> We started running into issues after bringing in the changes of the upcoming
> BR 2021.02 release, because raw output with the feature enabled looks like
> this, with interspersed escape sequences like "<ESC>.[?2004h":
[--SNIP--]
> Our automated test setup didn't like that too much. And others might be
> facing similar issues.

Yup, I can see where that would hurt...

[--SNIP--]
> diff --git a/package/readline/Config.in b/package/readline/Config.in
> index 7021472623c8..ed63e00cc8f3 100644
> --- a/package/readline/Config.in
> +++ b/package/readline/Config.in
> @@ -7,3 +7,17 @@ config BR2_PACKAGE_READLINE
>  	  as they are typed in.
>  
>  	  https://tiswww.case.edu/php/chet/readline/rltop.html
> +
> +config BR2_PACKAGE_READLINE_BRACKETED_PASTE
> +	bool "Enable bracketed paste"
> +	depends on BR2_PACKAGE_READLINE
> +	help
> +	  Enable the "bracketed paste" feature in libreadline. Bracketed paste
> +	  is helpful for interactive sessions when one wants to prevent pasted
> +	  text from being interpreted as typed-in commands. However, it also
> +	  causes control characters to show up in the raw output of a (telnet)
> +	  session. This can cause issues and throw off pattern matching if the
> +	  session output is being captured for automated processing.
> +	  See
> +	      https://cirw.in/blog/bracketed-paste
> +	  for further information on this feature and whether you may want it.

    $ make check-package
    package/readline/Config.in:15: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/readline/Config.in:16: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/readline/Config.in:17: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/readline/Config.in:18: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/readline/Config.in:19: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/readline/Config.in:23: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)

This is too bad, because you had managed to have a very nicely justified
block of help text that I foudn really good to look at... Alas, it does
not match the coding rules... <Sob...>

> diff --git a/package/readline/readline.mk b/package/readline/readline.mk
> index 326cffab1880..ce2a08b9375b 100644
> --- a/package/readline/readline.mk
> +++ b/package/readline/readline.mk
> @@ -16,6 +16,10 @@ READLINE_LICENSE = GPL-3.0+
>  READLINE_LICENSE_FILES = COPYING
>  READLINE_CPE_ID_VENDOR = gnu
>  
> +ifeq ($(BR2_PACKAGE_READLINE_BRACKETED_PASTE),)
> +	READLINE_CONF_OPTS += --disable-bracketed-paste-default
> +endif

No indentation in conditional blocks in makefiles (yup, that' goes counter
best practices, I'm not fan either, but that's what we do everywhere
else (mostly), and consistency wins.)

Also, we want explicit enable/disable, and we prefer positive logic:

    ifeq ($(BR2_PACKAGE_READLINE_BRACKETED_PASTE),y)
    READLINE_CONF_OPTS += --enable-bracketed-paste-default
    else
    READLINE_CONF_OPTS += --disable-bracketed-paste-default
    endif

Applied to master with the above fixed, thanks.

Regards,
Yann E. MORIN.

> +
>  define READLINE_INSTALL_INPUTRC
>  	$(INSTALL) -D -m 644 package/readline/inputrc $(TARGET_DIR)/etc/inputrc
>  endef
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Markus Mayer Feb. 26, 2021, 10:19 p.m. UTC | #2 
On Fri, 26 Feb 2021 at 13:43, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Markus, All,
>
> > +
> > +config BR2_PACKAGE_READLINE_BRACKETED_PASTE
> > +     bool "Enable bracketed paste"
> > +     depends on BR2_PACKAGE_READLINE
> > +     help
> > +       Enable the "bracketed paste" feature in libreadline. Bracketed paste
> > +       is helpful for interactive sessions when one wants to prevent pasted
> > +       text from being interpreted as typed-in commands. However, it also
> > +       causes control characters to show up in the raw output of a (telnet)
> > +       session. This can cause issues and throw off pattern matching if the
> > +       session output is being captured for automated processing.
> > +       See
> > +           https://cirw.in/blog/bracketed-paste
> > +       for further information on this feature and whether you may want it.
>
>     $ make check-package
>     package/readline/Config.in:15: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
>     package/readline/Config.in:16: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
>     package/readline/Config.in:17: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
>     package/readline/Config.in:18: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
>     package/readline/Config.in:19: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
>     package/readline/Config.in:23: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
>
> This is too bad, because you had managed to have a very nicely justified
> block of help text that I foudn really good to look at... Alas, it does
> not match the coding rules... <Sob...>

I'll try to remember the 72 column limit for next time.

> > diff --git a/package/readline/readline.mk b/package/readline/readline.mk
> > index 326cffab1880..ce2a08b9375b 100644
> > --- a/package/readline/readline.mk
> > +++ b/package/readline/readline.mk
> > @@ -16,6 +16,10 @@ READLINE_LICENSE = GPL-3.0+
> >  READLINE_LICENSE_FILES = COPYING
> >  READLINE_CPE_ID_VENDOR = gnu
> >
> > +ifeq ($(BR2_PACKAGE_READLINE_BRACKETED_PASTE),)
> > +     READLINE_CONF_OPTS += --disable-bracketed-paste-default
> > +endif
>
> No indentation in conditional blocks in makefiles (yup, that' goes counter
> best practices, I'm not fan either, but that's what we do everywhere
> else (mostly), and consistency wins.)
>
> Also, we want explicit enable/disable, and we prefer positive logic:
>
>     ifeq ($(BR2_PACKAGE_READLINE_BRACKETED_PASTE),y)
>     READLINE_CONF_OPTS += --enable-bracketed-paste-default
>     else
>     READLINE_CONF_OPTS += --disable-bracketed-paste-default
>     endif
>
> Applied to master with the above fixed, thanks.

And I'll try to remember this, as well.

Thanks,
-Markus
diff mbox series

Patch

diff --git a/package/readline/Config.in b/package/readline/Config.in
index 7021472623c8..ed63e00cc8f3 100644
--- a/package/readline/Config.in
+++ b/package/readline/Config.in
@@ -7,3 +7,17 @@  config BR2_PACKAGE_READLINE
 	  as they are typed in.
 
 	  https://tiswww.case.edu/php/chet/readline/rltop.html
+
+config BR2_PACKAGE_READLINE_BRACKETED_PASTE
+	bool "Enable bracketed paste"
+	depends on BR2_PACKAGE_READLINE
+	help
+	  Enable the "bracketed paste" feature in libreadline. Bracketed paste
+	  is helpful for interactive sessions when one wants to prevent pasted
+	  text from being interpreted as typed-in commands. However, it also
+	  causes control characters to show up in the raw output of a (telnet)
+	  session. This can cause issues and throw off pattern matching if the
+	  session output is being captured for automated processing.
+	  See
+	      https://cirw.in/blog/bracketed-paste
+	  for further information on this feature and whether you may want it.
diff --git a/package/readline/readline.mk b/package/readline/readline.mk
index 326cffab1880..ce2a08b9375b 100644
--- a/package/readline/readline.mk
+++ b/package/readline/readline.mk
@@ -16,6 +16,10 @@  READLINE_LICENSE = GPL-3.0+
 READLINE_LICENSE_FILES = COPYING
 READLINE_CPE_ID_VENDOR = gnu
 
+ifeq ($(BR2_PACKAGE_READLINE_BRACKETED_PASTE),)
+	READLINE_CONF_OPTS += --disable-bracketed-paste-default
+endif
+
 define READLINE_INSTALL_INPUTRC
 	$(INSTALL) -D -m 644 package/readline/inputrc $(TARGET_DIR)/etc/inputrc
 endef