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 region comment support for vi-mode and region commands #1176

Merged
merged 9 commits into from
Jan 13, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions extensions/vi-mode/visual.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,28 @@
(move-to-column *start-point* end-col)
(move-to-column (current-point) start-col))
(vi-visual-swap-points)))

(defmethod lem/language-mode:set-region-point ((start point) (end point) (global-mode (eql :|vi|)))
(declare (ignore global-mode))
(when (visual-p)
(let ((v-range (visual-range)))
(move-point start (car v-range))
(move-point end (cadr v-range)))))


(eval-when (:compile-toplevel :load-toplevel)
(sb-ext:unlock-package :lem-base)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we want a SBCL feature flag? Or see https://quickdocs.org/cl-package-locks which defines lock/unlock for 4 implementations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh indeed, nice catch, I'm not sure If I want to add another package, but I'll use a flag for that expression 👍


(defmethod lem-base::global-mode-region-beginning ((global-mode (eql :|vi|)) &optional (buffer (current-buffer)))
(declare (ignore buffer))
(if (visual-p)
(car (visual-range))
(editor-error "Not in visual mode")))

(defmethod lem-base::global-mode-region-end ((global-mode (eql :|vi|)) &optional (buffer (current-buffer)))
(declare (ignore buffer))
(if (visual-p)
(cadr (visual-range))
(editor-error "Not in visual mode")))

(sb-ext:lock-package :lem-base))
2 changes: 1 addition & 1 deletion lem.asd
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@
(:file "popup")
(:file "modeline")
(:file "command")
(:file "defcommand")
(:file "mode")
(:file "defcommand")
(:file "keymap")
(:file "event-queue")
(:file "interp")
Expand Down
10 changes: 10 additions & 0 deletions src/base/basic.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,21 @@ The thrid argument PROP is a property to remove."
(count-characters (buffer-start-point buffer)
(buffer-end-point buffer))))

(defgeneric global-mode-region-beginning (global-mode &optional buffer))
Copy link
Member

Choose a reason for hiding this comment

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

The concept of global-mode does not exist at the time of loading lem-base.
This causes interdependencies between lem-base and lem-core.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's why I'm using keywords to dispatch the global modes, as in this stage lem/base doesn't have the concept yet. But we can always work with keywords.


(defmethod global-mode-region-beginning ((global-mode (eql :|emacs|)) &optional (buffer (current-buffer)))
(region-beginning buffer))

(defun region-beginning (&optional (buffer (current-buffer)))
"Return the integer value of point or mark, whichever is smaller."
(point-min (buffer-point buffer)
(buffer-mark buffer)))

(defgeneric global-mode-region-end (global-mode &optional buffer))

(defmethod global-mode-region-end ((global-mode (eql :|emacs|)) &optional (buffer (current-buffer)))
(region-end buffer))

(defun region-end (&optional (buffer (current-buffer)))
"Return the integer value of point or mark, whichever is larger."
(point-max (buffer-point buffer)
Expand Down
2 changes: 2 additions & 0 deletions src/base/package.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@
:insert-string
:delete-character
:erase-buffer
:global-mode-region-beginning
:region-beginning
:global-mode-region-end
:region-end
:map-region
:points-to-string
Expand Down
14 changes: 10 additions & 4 deletions src/defcommand.lisp
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
(in-package :lem-core)


(defun maybe-marked ()
(when (eql (current-global-mode-keyword-name) :|emacs|)
(check-marked)))

(eval-when (:compile-toplevel :load-toplevel)
(defun parse-arg-descriptors (arg-descriptors universal-argument)
"Parse arg descriptors given to define-command.
Expand Down Expand Up @@ -50,10 +55,11 @@
:default nil
:existing nil)))
(#\r
(push '(check-marked) pre-forms)
'(list
(region-beginning)
(region-end)))))
(push '(maybe-marked) pre-forms)
'(list (global-mode-region-beginning
(current-global-mode-keyword-name))
(global-mode-region-end
(current-global-mode-keyword-name))))))
((and (consp arg-descriptor)
(eq :splice (first arg-descriptor)))
(assert (alexandria:length= arg-descriptor 2))
Expand Down
16 changes: 10 additions & 6 deletions src/ext/language-mode.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
:display-xref-locations
:display-xref-references
:find-root-directory
:buffer-root-directory)
:buffer-root-directory
:set-region-point)
#+sbcl
(:lock t))
(in-package :lem/language-mode)
Expand Down Expand Up @@ -153,8 +154,11 @@
(uncomment-region)
(comment-region)))

(defun set-region-point (start end)
(cond
(defgeneric set-region-point (start end global-mode))

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to define a new method (e.g. set-region-point-using-global-mode) that takes global-mode as an argument, with set-region-point as the default value. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, we can change the name of the function to reflect it, and at this point we can use the global-mode symbol so that's a good option too

(defmethod set-region-point ((start point) (end point) (global-mode (eql :|emacs|)))
(declare (ignore global-mode))
(cond
((buffer-mark-p (current-buffer))
(move-point start (cursor-region-beginning (current-point)))
(move-point end (cursor-region-end (current-point))))
Expand All @@ -166,7 +170,7 @@
(alexandria:when-let ((line-comment (variable-value 'line-comment :buffer)))
(with-point ((start (current-point))
(end (current-point)))
(set-region-point start end)
(set-region-point start end (lem-core::current-global-mode-keyword-name))
(loop
(skip-whitespace-forward start)
(when (point>= start end)
Expand All @@ -183,7 +187,7 @@
(when line-comment
(with-point ((start (current-point) :right-inserting)
(end (current-point) :left-inserting))
(set-region-point start end)
(set-region-point start end (lem-core::current-global-mode-keyword-name))
(skip-whitespace-forward start)
(when (point>= start end)
(insert-string (current-point) line-comment)
Expand All @@ -210,7 +214,7 @@
(when line-comment
(with-point ((start (current-point) :right-inserting)
(end (current-point) :right-inserting))
(set-region-point start end)
(set-region-point start end (lem-core::current-global-mode-keyword-name))
(let ((p start))
(loop
(parse-partial-sexp p end nil t)
Expand Down
4 changes: 4 additions & 0 deletions src/mode.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@
(defun current-global-mode ()
*current-global-mode*)

(defun current-global-mode-keyword-name ()
(alexandria:make-keyword
(mode-name (current-global-mode))))
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to use current-global-mode-keyword-name instead of current-global-mode?

Copy link
Member Author

Choose a reason for hiding this comment

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


(defun all-active-modes (buffer)
(mapcar #'ensure-mode-object
(append (buffer-minor-modes buffer)
Expand Down