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

Add lem:this-command-keys and lem:universal-argument-of-this-command #986

Merged
merged 4 commits into from
Aug 22, 2023

Conversation

fukamachi
Copy link
Collaborator

@fukamachi fukamachi commented Aug 21, 2023

This PR adds the following symbols:

  • lem:this-command-keys: A function to get a key sequence that invokes the current command.
  • lem:universal-argument-of-this-command: A function to get the universal argument of the current command.

These are equivalent to Emacs's this-command-keys and current-prefix-arg.

ref https://www.gnu.org/software/emacs/manual/html_node/elisp/Command-Loop-Info.html#index-this_002dcommand_002dkeys

Originally, I was making these changes to implement vi-mode's ., an operator to repeat the last operation, however, I split them as an individual PR to hear other's thoughts.

@fukamachi fukamachi changed the title Add lem:this-command-keys and `lem:*current-prefix-arg* Add lem:this-command-keys and lem:*current-prefix-arg* Aug 21, 2023
src/command.lisp Outdated
Comment on lines 14 to 15
(defvar *current-prefix-arg* nil
"The raw prefix argument for the current command.")
Copy link
Member

Choose a reason for hiding this comment

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

It may be a good idea to wrap this variable in a function as well for future refactoring.

Copy link
Member

Choose a reason for hiding this comment

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

We call this a universal-argument, so I would avoid adding a new term, prefix-arg.
How about the name *universal-argument* and (universal-argument-of-last-command) ?
If you have any other good names, please let me know.

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 for your comment!
Yeah, "naming" is one of the reasons I wanted to ask you because we don't need to have the same name as Emacs has.
I'll change the variable to have universal-argument in its name.

It may be a good idea to wrap this variable in a function as well for future refactoring.

Actually, I'm not sure whether it should be wrapped with a function.
If it's exposed as a special variable, it could be convenient when people want it to be changed in a lexical scope, like:

(let ((*universal-argument* 10))
  (call-command 'something nil))

To be clear, I'm not confident with it, so I'd like to hear if there's some kind of rule (or habit) in Lem to do, or if you prefer it for some reason.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It could be a reason to wrap by a function that the variable is used differently in some contexts.
For example, as you mentioned, universal-argument-of-last-command implies there's also this-universal-argument or so.

Copy link
Member

@cxxxr cxxxr Aug 22, 2023

Choose a reason for hiding this comment

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

If it's exposed as a special variable, it could be convenient when people want it to be changed in a lexical scope, like:

In this case, the second argument of call-command is universal-argument, so binding of the special variable appears unnecessary. (but with a little more complex code, it could certainly be useful.)

Wrapping a function makes it easier to insert breaks at the point of call, and I believe it increases maintainability.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, at least in my use case, it's not necessary to be a special variable, so I'll go with a function style.

@fukamachi fukamachi requested a review from cxxxr August 22, 2023 11:56
@fukamachi fukamachi changed the title Add lem:this-command-keys and lem:*current-prefix-arg* Add lem:this-command-keys and lem:universal-argument-of-this-command Aug 22, 2023
@cxxxr
Copy link
Member

cxxxr commented Aug 22, 2023

Thank you.
this change improves Lem's API.

@cxxxr cxxxr merged commit 775e9b3 into lem-project:main Aug 22, 2023
1 check passed
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.

2 participants