Re: [PATCH 1/2] vt: bracketed paste support

[Jiri Slaby <jirislaby@kernel.org>]

On 14. 05. 25, 3:52, Nicolas Pitre wrote:
> From: Nicolas Pitre <npitre@baylibre.com>
> 
> This is comprised of 3 aspects:
> 
> - Take note of when applications advertise bracketed paste support via
>    "\e[?2004h" and "\e[?2004l".
> 
> - Insert bracketed paste markers ("\e[200~" and "\e[201~") around pasted
>    content in paste_selection() when bracketed paste is active.
> 
> - Add TIOCL_GETBRACKETEDPASTE to return bracketed paste status so user
>    space daemons implementing cut-and-paste functionality (e.g. gpm,
>    BRLTTY) may know when to insert bracketed paste markers.
> 
> Link: https://en.wikipedia.org/wiki/Bracketed-paste

LGTM.

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

In case (and only then) you resend for some reason, I have some nits below.

(And a question at the bottom.)

> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> ---
>   drivers/tty/vt/selection.c     | 35 ++++++++++++++++++++++++++++++----
>   drivers/tty/vt/vt.c            | 15 +++++++++++++++
>   include/linux/console_struct.h |  1 +
>   include/uapi/linux/tiocl.h     |  1 +
>   4 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
> index 791e2f1f7c0b..ac86b82411a8 100644
> --- a/drivers/tty/vt/selection.c
> +++ b/drivers/tty/vt/selection.c
...
> @@ -427,10 +433,31 @@ int paste_selection(struct tty_struct *tty)
>   			continue;
>   		}
>   		__set_current_state(TASK_RUNNING);
> +
> +		if (bps) {
> +			count = tty_ldisc_receive_buf(ld, bps, NULL, strlen(bps));
> +			bps += count;
> +			if (*bps == '\0')
> +				bps = NULL;
> +			else
> +				continue;

This could have been simpler:

bps += tty_ldisc_receive_buf(ld, bps, NULL, strlen(bps));
if (*bps != '\0')
   continue;

bps = NULL;

> +		}
> +
>   		count = vc_sel.buf_len - pasted;
> -		count = tty_ldisc_receive_buf(ld, vc_sel.buffer + pasted, NULL,
> -					      count);
> -		pasted += count;
> +		if (count) {
> +			count = tty_ldisc_receive_buf(ld, vc_sel.buffer + pasted,
> +						      NULL, count);
> +			pasted += count;

Same here for pasted.

> +			if (vc_sel.buf_len > pasted)
> +				continue;
> +		}
> +
> +		if (bpe) {
> +			count = tty_ldisc_receive_buf(ld, bpe, NULL, strlen(bpe));
> +			bpe += count;

And bpe.

> +			if (*bpe == '\0')
> +				bpe = NULL;
> +		}
>   	}
>   	mutex_unlock(&vc_sel.lock);
>   	remove_wait_queue(&vc->paste_wait, &wait);
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index efb761454166..ed39d9cb4432 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -1870,6 +1870,14 @@ int mouse_reporting(void)
>   	return vc_cons[fg_console].d->vc_report_mouse;
>   }
>   
> +/* invoked via ioctl(TIOCLINUX) */
> +static int get_bracketed_paste(struct tty_struct *tty)

vc_bracketed_paste is unsigned. (I understand tioclinux() returns an int.)

> +{
> +	struct vc_data *vc = tty->driver_data;
> +
> +	return vc->vc_bracketed_paste;
> +}
> +
>   enum {
>   	CSI_DEC_hl_CURSOR_KEYS	= 1,	/* CKM: cursor keys send ^[Ox/^[[x */
>   	CSI_DEC_hl_132_COLUMNS	= 3,	/* COLM: 80/132 mode switch */
...
> --- a/include/uapi/linux/tiocl.h
> +++ b/include/uapi/linux/tiocl.h
> @@ -36,5 +36,6 @@ struct tiocl_selection {
>   #define TIOCL_BLANKSCREEN	14	/* keep screen blank even if a key is pressed */
>   #define TIOCL_BLANKEDSCREEN	15	/* return which vt was blanked */
>   #define TIOCL_GETKMSGREDIRECT	17	/* get the vt the kernel messages are restricted to */
> +#define TIOCL_GETBRACKETEDPASTE	18	/* get whether paste may be bracketed */

Do you plan on updating man-pages too?

https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man/man2const/TIOCLINUX.2const

thanks,
-- 
js
suse labs