emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [ELPA] New package: shift-select


From: Stefan Monnier
Subject: Re: [ELPA] New package: shift-select
Date: Tue, 16 Apr 2019 18:43:18 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)

> (defun shift-select-pre-command-hook ()
>   "Shift select pre command hook."
>   (setq-local shift-select-shift-pressed

You already declared the var with `defvar-local` so you don't need
`setq-local` here (or alternatively, you can use plain `defvar` at
top-level and `setq-local` here).

>               (ignore-errors
>                 (and (stringp (symbol-name last-command-event))
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This either errors or returns t but never returns nil, sop you can just
remove it.  Tho I think what you meant to write was (symbolp
last-command-event), after which you should remove the `ignore-errors`
which would otherwise just hide programming errors.

>                      (string-match-p "S-" (symbol-name last-command-event)))))
>   (when (or this-command-keys-shift-translated
>             shift-select-shift-pressed)

When is this shift-select-shift-pressed needed?  It seems like this
might be working around a bug somewhere in the sense that
this-command-keys-shift-translated should have been non-nil already.

> (defun shift-select-post-command-hook ()
>   "Shift select post command hook."
>   (if (or this-command-keys-shift-translated
>           shift-select-shift-pressed)
>       (let ((cur-pt (point)))
>         ;; User moves the cursor when holding shift key.
>         (unless (= cur-pt shift-select-start-pt)

IIUC this is trying to notice when the command was a motion command.
Those should call `handle-shift-selection` (e.g. via the `^` char in
their interactive spec string) to announce that they're motion commands.
So IIUC this code tries to compensate for commands which fail to call
`handle-shift-selection`.

It's probably a good heuristic, but it would be good to report those
commands so we can fix them.  Using a heuristic like this one is OK for
an optional package, but for the core functionality we want something
that doesn't occasionally misfire (e.g. cur-pt can be different from
shift-select-start-pt because we switched to a different buffer, or
because text was inserted/deleted before point).

I don't see anything in this code which handles shift-selection
differently from the default code, so I'm not sure what is meant by
"Shift select text like how other text editor does".

Instead it seems to try and work around bugs in code which was (still)
not (yet) adjusted to cooperate with the "new" shift-selection
introduced in Emacs-23.  Maybe you could change the Commentary
accordingly (or otherwise clarifies in which way the behavior is
supposed to differ from the default shift-select-mode)?


        Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]