Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

evil-mode text insertion breaks emacs text property inheritance (stickiness of text properties) #1857

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dizzyyogi
Copy link

Issue type

  • Bug report

Environment

Emacs version: 29.0.91
Operating System: any
Evil version: 1.15.0
Evil installation type: straight.el + use-package
Graphical/Terminal: X
Tested in a make emacs session (see CONTRIBUTING.md): Yes

Reproduction steps

  • Start Emacs
  • Open a scratch buffer
  • Put some arbitrary text property on a whole line
    For example, from the following initial scratch buffer content:
    ==============================
    ;; This buffer is for text that is not saved, and for Lisp evaluation.
    ;; To create a file, visit it with C-x C-f and enter text in its buffer.
    ==============================
    Where position 72 corresponds to the first semi-column at the beginning of second line, and position 145 corresponds to the end of the second line
  • Execute:
    (put-text-property 72 145 'some-text-property t)
  • open a new line in evil normal state, with cursor placed somewhere on second line, and type some text:
    M-x evil-open-below ("o")
    Type "hello world"

Expected behavior

Because default stickiness of text properties is rear-sticky (see emacs manual, 33.19 Text Properties -> Sticky Properties), the text "hello world" and the newline character added after line 2 should have inherited the text property 'some-text-property set to t in the step 4 above.
By self-insertion of a new line character and "hello world" string in emacs state, the property is correctly inherited:
Place cursor on second line
C-z
C-e
TYpe Return key
Type "hello world"
Place cursor on any of the inserted characters
Use command what-cursor-position with universal arg to get the text properties at point:
C-u C-x =
Any of the inserted characters have inherited property 'some-text-property, because of default rear-stickiness rule.

Actual behavior

After evil-open-below + text insertion
The text property 'some-text-property is not attached to the inserted text "hello world".

Further notes

The functions evil-insert-newline-above/below use (insert "\n").
As indicated in elisp manual, 33.19.6 Stickiness of Text Properties":
"The ordinary text insertion functions, such as 'insert', do not inherit any properties. They insert text with precisely the properties of the string being inserted, and no others."
Shouldnt' function "insert-and-inherit" be used instead?

@tomdl89
Copy link
Member

tomdl89 commented Jan 19, 2024

Thanks for the bug report @dizzyyogi. I think this is an oversight rather than intentional, so I'd welcome a PR. I guess you'd change evil-insert-newline-above too, to pick up the properties of the preceding line (or current line if front-sticky)?

@ydutrieux
Copy link

ydutrieux commented Jan 19, 2024

Many thanks Tom for your prompt reply.

Indeed I would at a minimum also change evil-insert-newline-above too, to
simply rely on normal Emacs text property inheritance rules, how ever
front-sticky and rear-nonsticky properties are set.

But I was in fact also wondering about any call to insert/insert-char functions
in the whole evil code base, because in principle I think evil insertion or
replacement/paste triggered from a user edition command should behave like
normal user originated self-insert/yank in emacs state.

It seems the following functions/commands are potentially impacted:

  • evil-put
  • evil-copy
  • evil-move
  • evil-invert-case
  • evil-replace
  • evil-visual-paste
  • evil-insert-digraph
  • evil-read
  • evil-replace-backspace
  • evil-yank-line-handler
  • evil-yank-block-handler
  • evil-execute-change

From the above list, I would think those functions/commands should be fixed:

  • evil-copy
  • evil-move
  • evil-invert-case
  • evil-replace
  • evil-visual-paste
  • evil-insert-digraph

This should probably not be changed:

  • evil-put

And those I am not sure:

  • evil-read
  • evil-replace-backspace
  • evil-yank-line-handler
  • evil-yank-block-handler
  • evil-execute-change

I'd definitely like to help, but given I'm fairly new to Emacs and completely
to Emacs package contribution, I feel this would probably need more expert eyes
than mine to assess the situation and risks of changing insert or insert-char
by insert-and-inherit in each of those cases.

Or should I submit a PR only for evil-insert-newline-above/below to start with?

@tomdl89
Copy link
Member

tomdl89 commented Jan 23, 2024

Thanks for the research! I agree with your list of commands that should be fixed, plus I think evil-ex-put (I don't think evil-put exists) should be changed too. The comment about insert in that function is just making the point that yank-handlers should be ignored. insert-and-inherit should do that too. I also think the ones you're not sure about are fine to change, except for evil-replace-backspace which should be inserting text (properties or not) that is stored. I think go ahead with a PR if you have some use-cases of this functionality in mind. I.e. we can write some meaningful tests for the new behaviour.

This leads me to ask, what are you trying to do, which the current insert -only functionality is breaking? I always prefer to change stable codebases like evil if there's a practical reason, rather than just code correctness.

@dizzyyogi
Copy link
Author

Hi.
Regarding your question "what are you trying to do, which the current
insert -only functionality is breaking?":

I am trying to provide regexp-based buffer delimitation into cells, cell
fontification and navigation based on text properties, font lock managed text
properties, and text property search.

This might turn into some package I'd love to share with the community,
although I feel at this point there is still quite some way to go before this
would be sufficiently mature to be shareable...

I noticed that functionalities working in vanilla Emacs were broken with
evil-mode.
For example, the text property "line-prefix", which allows to provide a
left-border to the cells, was lost when opening a line above or below the
current line. Whereas according to property inheritance rules, the line-prefix
property should have been preserved for the new opened line.
Similarly, I am using a custom text property to "watermark" text of a given cell,
allowing to delimit and navigate cells based on text property search, but
continuity of the cells was broken when inserting lines or even some character,
due to the text properties not being properly inherited upon simple text insertion.

@tomdl89
Copy link
Member

tomdl89 commented Jan 23, 2024

That's an excellent answer. Please feel free to go ahead with a PR for the functions discussed.

- evil-ex-put
- evil-copy
- evil-move
- evil-invert-case
- evil-replace
- evil-visual-paste
- evil-insert-digraph
- evil-read
- evil-replace-backspace
- evil-yank-line-handler
- evil-yank-block-handler
- evil-execute-change
evil-states.el Outdated Show resolved Hide resolved
evil-commands.el Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants