drym-org / symex.el

An intuitive way to edit Lisp symbolic expressions ("symexes") structurally in Emacs
Other
271 stars 22 forks source link

Inconsistent visual line movement around comment lines #91

Open olnw opened 1 year ago

olnw commented 1 year ago

The functions symex-previous-visual-line and symex-next-visual-line don't always move by visual line when point is below or above a comment line. In general, symex-next-visual-line will move to the next non-comment line, and symex-previous-visual-line will not move the cursor upwards if the above line is a comment.

For example, pressing g k with cursor (|) in this position does nothing:

 (message "Hello")
 ;; This is a comment.
 ;; This is another comment.
|(message "Hi!")

And when pressing g j with the following example, here is what happens:

|(message "Hello")
 ;; This is a comment.
 ;; This is another comment.
 (message "Hi!")
 (message "Hello")
 ;; This is a comment.
 ;; This is another comment.
|(message "Hi!")

Is this intended behaviour?

Here is a minimal reproducible example:

(require 'package)
(add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/") t)
(package-initialize)

(use-package symex
  :ensure t
  :config (symex-initialize)
  :bind ("s-;" . symex-mode-interface))

System Details

$ emacs --version
GNU Emacs 30.0.50
Development version 26740f30469c on master branch; build date 2023-03-09.
Copyright (C) 2023 Free Software Foundation, Inc.
GNU Emacs comes with ABSOLUTELY NO WARRANTY.
You may redistribute copies of GNU Emacs
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.
~/.emacs.d/straight/repos/symex.el $ git log -1 --format="%h %s"
29d69f6 Merge pull request #90 from olnw/add-symex-common-lisp-modes
countvajhula commented 1 year ago

I believe this is fixed on the 2.0-integration branch. Btw, the "macros" branch has been merged so if you'd like to try the new improvements you can use 2.0-integration now.

olnw commented 1 year ago

Hey again, and sorry for the late response (I've had a lot going on recently.) It almost works correctly on 2.0-integration, except in cases where a single logical line is too long to fit in one screen line.

For example, if the cursor is in this position, symex-next-visual-line will not move the cursor at all:

  "|this_is_a_really_long_line
↪ _of_text"

When it should be moving it to this position:

  "this_is_a_really_long_line
↪ _|of_text"

I think the problem is that symex-select-nearest-in-line is acting on logical lines when it should be acting on visual lines (it's only called by symex-next-visual-line and symex-previous-visual-line).

Here is a function to replace symex-select-nearest-in-line:

(defun symex-select-nearest-in-visual-line ()
  "Select symex nearest to point on the current visual line."
  (interactive)
  (unless (symex--current-visual-line-empty-p)
    (let ((original-pos (point))
          (visual-line-bounds
           (cons (save-excursion (beginning-of-visual-line) (point))
                 (save-excursion (end-of-visual-line) (point)))))
      (symex-select-nearest)
      (unless (range-member-p (point) visual-line-bounds)
        (goto-char original-pos)))))

symex--current-line-empty-p uses a regexp to check if the current logical line is empty. Here is a new function to check if the current visual line is empty:

(defun symex--current-visual-line-empty-p ()
  "Return non-nil if the current visual line is empty.
Otherwise, return nil."
  (save-excursion
    (let ((visual-line-start (progn (beginning-of-visual-line) (point)))
          (visual-line-end (progn (end-of-visual-line) (point))))
      (string-blank-p (buffer-substring-no-properties visual-line-start
                                                      visual-line-end)))))

Finally:

(defun symex-previous-visual-line (&optional count)
  "Coordinate navigation to move up.

This moves up COUNT lines in terms of buffer coordinates, rather than
structurally in terms of the tree."
  (interactive "p")
  (evil-previous-visual-line count)
Removed:  (symex-select-nearest-in-line))
  Added:  (symex-select-nearest-in-visual-line))
(defun symex-next-visual-line (&optional count)
  "Coordinate navigation to move down.

This moves down COUNT lines in terms of buffer coordinates, rather than
structurally in terms of the tree."
  (interactive "p")
  (evil-next-visual-line count)
Removed:  (symex-select-nearest-in-line))
  Added:  (symex-select-nearest-in-visual-line))

And with this, the issue seems to be resolved. Please let me know if I've done anything wrong.