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

Introduce Vim's options feature, and add :set & :cd #908

Merged
merged 13 commits into from
Aug 13, 2023

Conversation

fukamachi
Copy link
Collaborator

@fukamachi fukamachi commented Aug 13, 2023

Quick troubleshooting

Q. Why does the :e command no longer complete from the directory of the file being edited?

It is to make Lem's vi-mode closer to Vim's behavior. If you prefer the previous behavior, there's 2 options to achieve:

A1. Turn autochdir option on

Set autochdir to t in your init.lisp:

;; Originally, this code should work, but I found a problem later and fixed at #950 
;; (setf (lem-vi-mode:autochdir) t)

;; After #950 is merged
(setf (lem-vi-mode:vi-option-value "autochdir") t)

To change the option temporarily, just execute :set autochdir (and :set noautochdir to turn off).

A2. Change the current directory manually

Use :cd command to change the directory manually:

" Move to "extensions/vi-mode"
:cd extensions/vi-mode
" `%:h` expands to the directory of the current buffer's file
:cd %:h
" `-` means the previous directory
:cd -

Changes

  • Add Vim's options feature
    • Currently only autochdir is implemented
      • Works when running :e (find-file) or switching the buffer/window
  • [Incompatible changes] Change the base directory to find with :e to the current directory
    • You need to change the directory manually with :cd
      • :cd %:h moves to the current buffer's directory (See "filename modifiers" below)
    • Or, turn autochdir option on
  • Add "filename modifiers" which would be expanded in :e, :vs, :sp, and :cd
    • Examples:
      • %: The current buffer's file path
      • %:p: The absolute pathname of the current buffer's file path
      • %:h: The current buffer's directory
      • %:e: The file extension of the current buffer's file
      • %:t: The filename of the current buffer's file
      • %:r: The root of the filename
  • Add :cd for changing the current directory
    • :cd <dir> moves to the <dir>
    • :cd moves to the home directory
    • :cd - moves to the previous directory
  • Add :set (also :se works)
    • Examples:
      • Turn on: :set autochdir, :set acd, :set acd=t
      • Turn off: :set noautochdir, :set noacd, :set acd=nil
      • Toggle: :set invautochdir, :set autochdir!, :set invacd, :set acd!
      • Get the current value: :set autochdir, :set acd?
      • Reset to the default: :set autochdir&, :set acd&
    • In init.lisp
      • (setf (lem-vi-mode:autochdir) t)

@fukamachi fukamachi marked this pull request as draft August 13, 2023 02:24
@fukamachi
Copy link
Collaborator Author

fukamachi commented Aug 13, 2023

We may need filename modifiers, too, like :cd %:h to move to the buffer's directory.

@fukamachi fukamachi force-pushed the vi-mode-options branch 3 times, most recently from 8a7f79d to 1ca2a33 Compare August 13, 2023 07:16
@fukamachi fukamachi marked this pull request as ready for review August 13, 2023 08:04
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.

Thank you for the PR.
I made a few petty comments, but it seems like a very good change.

Comment on lines 158 to 162
(let ((lem-core::*message-timeout* 10))
(if (and isset
(not (equal option-value old-value)))
(lem:message "~A: ~S => ~S" option-name old-value option-value)
(lem:message "~A: ~S" option-name option-value)))))
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 show-message instead of message and message-timeout?

(lem:show-message (format ...) :timeout ...)

@@ -3,6 +3,7 @@
:lem
:lem/universal-argument)
(:import-from :cl-package-locks)
(:import-from #:cl-ppcre)
Copy link
Member

Choose a reason for hiding this comment

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

we use keywords for package-designator in this project, so it would be nice to see it unified.

Comment on lines +170 to +178
(lem:add-hook lem:*find-file-hook* 'auto-change-directory)
(dolist (window (lem:window-list))
(lem:add-hook (lem-core::window-switch-to-buffer-hook window) 'auto-change-directory)
(lem:add-hook (lem-core:window-leave-hook window) 'auto-change-directory)))
(progn
(lem:remove-hook lem:*find-file-hook* 'auto-change-directory)
(dolist (window (lem:window-list))
(lem:remove-hook (lem-core::window-switch-to-buffer-hook window) 'auto-change-directory)
(lem:remove-hook (lem-core:window-leave-hook window) 'auto-change-directory))))))
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps there is an issue that does not apply to windows that are split after this.
It might be a good idea to have the issue.

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 tried some cases on my machine, but it seems to be working. I don't know why.

(gethash name *options*)
(when (and (null exists)
error-if-not-exists)
(error "Unknown option: ~A" name))
Copy link
Member

Choose a reason for hiding this comment

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

error is handled as an internal error.
Instead of treating user input as an internal error, it may be better to use editor-error or a condition that inherits from it.

@cxxxr
Copy link
Member

cxxxr commented Aug 13, 2023

Please let me know when you are done making changes.

@fukamachi fukamachi requested a review from cxxxr August 13, 2023 14:15
@cxxxr
Copy link
Member

cxxxr commented Aug 13, 2023

I think it's good.
Thanks for the interesting PR.

@cxxxr cxxxr merged commit 1c4367c into lem-project:main Aug 13, 2023
1 check passed
fukamachi added a commit to fukamachi/lem that referenced this pull request Aug 14, 2023
Also, stop defining functions for each options, because 'number' conflicts with the 'cl' package.
(ref lem-project#908)
fukamachi added a commit to fukamachi/lem that referenced this pull request Aug 14, 2023
Also, stop defining functions for each option, because 'number' conflicts with the 'cl' package.
(ref lem-project#908)
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