auctex-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] fix delete-property handling


From: Ikumi Keita
Subject: Re: [PATCH] fix delete-property handling
Date: Tue, 09 Apr 2024 16:14:14 +0900

Hi Artem,

>>>>> Artem Yurchenko <artemyurchenko@zoho.com> writes:
> Hello,
> I am sending two patches that fix delete-property handling. They improve 
> interaction of auctex with other «electric» modes.

Thanks for your proposals. I'm now reading through the proposed codes.
They basically look good to me. I think that AUCTeX can accept them and
it requires copyright assignment. I assume that you haven't signed FSF
copyright assignment form before, so please follow this instruction if
you want to have your proposal incorporated into AUCTeX:
https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future

> This is my first time contributing and I expect to have made mistakes. I am 
> willing to react to comments and fix any deficiencies.

I'd like to comment on the first patch now; I haven't read the second
patch in detail yet. Commentary on the second would follow on another
day.

> +(defun TeX--put-electric-delete-selection (symbol electricp)
> +  "Set appropriate `delete-selection' property for electric functions.
> +
> +When the function bound to SYMBOL has «electric» behaviour, as
> +determined by ELECTRICP, `delete-selection' is set to nil.  In

I think it should explicitly state that ELECTRICP is actually a
predicate (function).

> +However, when /our/ electric behaviour is disabled (ELECTRICP is
> +nil), we want other electric modes to operate freely.

Shouldn't it be "ELECTRICP returns nil" instead of "ELECTRICP is nil"?
It seems as if ELECTRICP was just an boolean flag rather than a
predicate.

(This isn't a suggestion, just an impression.)
> +  (put symbol 'delete-selection
> +       (lambda ()
> +         (unless (funcall electricp)
> +           (get #'self-insert-command 'delete-selection)))))

Good simple solution. I wasn't aware that `delete-selection' property
can return a function because of the commentary of delsel.el:
;;  FUNCTION
;;      For commands which need to dynamically determine this behavior.
;;      FUNCTION should take no argument and return one of the above
;;      values, or nil.  In the latter case, FUNCTION should itself
;;      do with the active region whatever is appropriate."
I took that "one of the above values" doesn't include FUCTTION itself.
However, `delete-selection-helper' is actually written to operate
recursively, which I didn't realize.
(That was the reason I wrote the current `delete-selection' property of
`LaTeX-insert-left-brace' to discriminate the cases between '(yank
supersede kill t nil) and a function.)

(This is optional.)
> +(defun LaTeX-insert-left-brace-electric ()
[...]
> +  (let ((lbrace (char-to-string last-command-event)) lmacro skip-p)
[...]
> +    (insert last-command-event)

Since `LaTeX-insert-left-brace-electric' is no longer a command bound to
a key, I'm inclined to writing it to receive `last-command-event' as an
argument of the function rather than to refer to the variable directly.

Regards,
Ikumi Keita
#StandWithUkraine #StopWarInUkraine
#Gaza #StopMassiveKilling #CeasefireNOW



reply via email to

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