Re: Set X primary selection with Emacs in xterm

[Duncan Findlay <duncf@google.com>]

Thanks for the feedback!

On Thu, Jun 2, 2022 at 10:34 PM Po Lu <luangruo@yahoo.com> wrote:
>
> (Patches should ideally be sent to bug-gnu-emacs@gnu.org, and not
> here.)

I've sent a new version to the bug tracker:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55883

> Duncan Findlay <duncf@google.com> writes:
>
> > I frequently use Emacs over ssh and I'd really like to get both
> > primary and clipboard selections to work as close as possible to
> > running Emacs on X natively. I'd like to kill text in Emacs and have
> > that show up in my system clipboard so I can paste into other
> > applications. Similarly, if I select text with mark and keyboard (or
> > mouse with xterm-mouse-mode), I'd like it to update my local X's
> > primary selection so I can middle-click to paste it elsewhere. I have
> > two patches attached that got this working for me.
> >
> > Without changes, with `(setq xterm-extra-capabilities
> > '(setSelection))', when I kill text, Emacs generates OSC 52 terminal
> > escape codes and xterm updates my clipboard. This works great! Emacs
> > also has support for updating the primary selection with this same
> > mechanism, e.g. `(gui-set-selection 'PRIMARY "primary")'. This, too,
> > works fine with xterm.
> >
> > The bit that's missing is that when I select text with keyboard or
> > mouse (with xterm-mouse-mode), the primary selection is not updated.
> > It appears that the primary selection is only updated when
> > `(window-system)' is not nil.
> >
> > I've attached a patch below to replace the `window-system' check with
> > `display-selections-p', as that's documented as the preferred way to
> > do this type of check. It also moves the check to lisp where we can
> > advise it.
> >
> > The second patch changes  `(display-selections-p)' to return true
> > under xterm with the setSelection feature enabled.
>
> Thanks.  That mostly looks good to me, some minor comments below:
>
> >  * src/keyboard.c (command_loop_1): Replace call to `window-system'
> >  with `display-selections-p' when deciding whether to update primary
> >  selection.
>
> >  * lisp/frame.el (display-selections-p): Return `t' when xterm's
> >  setSelection mode is enabled.
>
> We don't indent each line of a ChangeLog entry to line up with the
> asterisk, so this should be written:
>
>  * src/keyboard.c (command_loop_1): Replace call to
> `window-system' with `display-selections-p' when deciding
> whether to update primary selection.
>
> instead.  Please also make sure that each line of the commit message
> takes no more than 64 columns on-screen.

I'd missed reading CONTRIBUTE, where this is all documented. Thank you.

>
> >  lisp/frame.el | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/lisp/frame.el b/lisp/frame.el
> > index 27f99fb7d2..eb8ae1c27d 100644
> > --- a/lisp/frame.el
> > +++ b/lisp/frame.el
> > @@ -2164,6 +2164,9 @@ display-selections-p
> >         (not (null dos-windows-version))))
> >       ((memq frame-type '(x w32 ns pgtk))
> >        t)
> > +     ((and (memq frame-type '(t))
>
> This `memq' is redundant: why not (eq frame-type t) instead?

Oops, thank you. (I'm very new to lisp and mostly pattern matching.)

>
> > +           (eq (terminal-parameter nil 'xterm--set-selection) t))
> > +      t)
>
> This doesn't look very clean... I wonder if there is a cleaner way to
> check for this support.

I figured folks would probably object to that. Based on the responses
here, and given that this is only needed for text-based terminals, I
think using a generically-named terminal parameter that can be set by
any other library makes the most sense.

> Also, have you signed copyright papers?  And if not, how many lines of
> code have you previously contributed to Emacs?

This should be covered by my employer's copyright paperwork on file.

Thanks
Duncan