Skip to content

Comments

Refactor definitions of vi-mode operators.#952

Merged
cxxxr merged 14 commits intolem-project:mainfrom
fukamachi:vi-mode-operator-refactor
Aug 17, 2023
Merged

Refactor definitions of vi-mode operators.#952
cxxxr merged 14 commits intolem-project:mainfrom
fukamachi:vi-mode-operator-refactor

Conversation

@fukamachi
Copy link
Collaborator

@fukamachi fukamachi commented Aug 15, 2023

Add define-vi-motion and define-vi-operator macros, and rewrite vi-mode's operator commands with them.

This aims to make them much cleaner and behave more consistently.

Bugs fixed

  • Fix dh, ch, d^ and c^ not to delete the char under the cursor
  • Fix 1000dl to delete the last char of the line
  • Fix df<char>, cf<char>, dt<char>, and ct<char> to delete the char under the cursor after the movement
  • Fix dF<char>, dT<char> not to delete the char under the cursor
  • Fix cj to remain a single line
  • Fix yy to allow to take a universal argument
  • Fix J not to skip empty lines, and don't allow to concatenate the following ones
  • Fix J and gJ to allow to take a universal argument, like 3J to join 3 lines
  • Fix J not to add a space in between if the next line starts with a ')' not only when the current line ends with ')'
  • Fix J in visual mode to join lines between pointers
  • Fix D in visual mode to delete the whole lines between pointers
  • Fix r to work in visual mode
  • Fix r to allow to take a universal argument, like 2r<char>
  • Fix the cursor position when s is pressed on the last char of the line
  • Fix dG to delete to the end of the buffer instead to the begin
  • Fix dM and dL to delete the line if the pointer is the end of the buffer after the movement
  • Add gU and gu to change cases in normal mode
  • ...and more, fix a lot of operations that move the cursor to the wrong position after they run

I also found strange behavior of e and E, but I decided to leave them not to make changes huge.

@fukamachi fukamachi force-pushed the vi-mode-operator-refactor branch 7 times, most recently from 0b3670f to 09510cf Compare August 15, 2023 21:51
@fukamachi fukamachi force-pushed the vi-mode-operator-refactor branch 6 times, most recently from ae964a3 to 3adb7f3 Compare August 16, 2023 02:39
@fukamachi fukamachi changed the title [WIP] Refactor definitions of vi-mode operators. Refactor definitions of vi-mode operators. Aug 16, 2023
@fukamachi fukamachi marked this pull request as ready for review August 16, 2023 02:45
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed define-command to allow to take :initargs to specify :default-initargs to the class.

I don't believe this breaks any other parts, but please keep in mind.

@fukamachi fukamachi marked this pull request as draft August 16, 2023 11:58
:delete-point
:point-buffer
:point-charpos
:point-linum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using line-number-at-point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I rewrote it with the function and stopped exporting.

@fukamachi fukamachi force-pushed the vi-mode-operator-refactor branch from 2bf4278 to 8c92c61 Compare August 16, 2023 13:12
@fukamachi fukamachi requested a review from cxxxr August 17, 2023 01:39
@fukamachi fukamachi marked this pull request as ready for review August 17, 2023 01:41
@fukamachi fukamachi force-pushed the vi-mode-operator-refactor branch from 0ad1354 to 578023e Compare August 17, 2023 05:58
Copy link
Member

@cxxxr cxxxr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a very nice change.
One thing that concerns me is that there are several compile-time warnings, could you please fix them?

:type ,(or type :exclusive)
:default-n-arg ,default-n-arg))
,arg-list ,arg-descriptor
(with-point ((*vi-origin-point* (current-point)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits:
with-point creates a point with kind = temporary by default.
This means that it will become invalid after the buffer has been modified, so care should be taken in the future.

https://github.com/lem-project/lem/blob/main/src/base/point.lisp#L20

"Goto end of a line."
(line-end point)
(unless (bolp point)
(character-offset point *cursor-offset*)))
Copy link
Member

@cxxxr cxxxr Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better to move before using definitions. *cursor-offset*

@fukamachi fukamachi force-pushed the vi-mode-operator-refactor branch from 578023e to 3e261c9 Compare August 17, 2023 09:32
@cxxxr cxxxr merged commit 8647130 into lem-project:main Aug 17, 2023
@cxxxr
Copy link
Member

cxxxr commented Aug 17, 2023

Thank you for so many improvements!

cxxxr added a commit that referenced this pull request Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants