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 test for hargs:sexpression-p #451

Merged
merged 2 commits into from
Jan 20, 2024

Conversation

matsl
Copy link
Collaborator

@matsl matsl commented Jan 20, 2024

What

Add test for hargs:sexpression-p.

Why

When reading through the doc string it struck me that it was not a perfect match for the functionality. So I decided to write some tests to verify the behavior. Here it is.

What looked wrong is that the doc string says the following:

If point follows an sexpression end character, the preceding sexpression is returned.  
If point precedes an sexpression start character, the following sexpression is returned.  
Otherwise, the innermost sexpression

As can be seen from the test the cases for point preceding the start character does not seems to work as expected.

Then I noticed that this function is only used in one place so maybe overkill to spent to much time on it but it felt like a generic function so might be worth fixing anyway!?

@matsl matsl requested a review from rswgnu January 20, 2024 11:23
Copy link
Owner

@rswgnu rswgnu left a comment

Choose a reason for hiding this comment

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

You are right, there was a bug in hargs:sexpression-p. To fix your PR here, you'll have to replace that function with the version below and then update your test function with the version below as well.

(defun hargs:sexpression-p (&optional no-recurse)
  "Return an sexpression at point as a string.
If point follows an sexpression end character, the preceding sexpression
is returned.  If point precedes an sexpression start character, the
following sexpression is returned.  Otherwise, the innermost sexpression
that point is within is returned or nil if none."
  (let ((not-quoted
	 '(condition-case ()
	      (not (and (eq (char-syntax (char-after (- (point) 2))) ?\\)
			(not (eq (char-syntax (char-after (- (point) 3))) ?\\))))
	    (error t))))
    (save-excursion
      (ignore-errors
	(cond ((and (eq (char-syntax (preceding-char)) ?\))
		    ;; Ignore quoted end chars.
		    (eval not-quoted))
	       (buffer-substring (point)
				 (progn (forward-sexp -1) (point))))
	      ((and (eq (char-syntax (following-char)) ?\()
		    ;; Ignore quoted begin chars.
		    (eval not-quoted))
	       (buffer-substring (point)
				 (progn (forward-sexp) (point))))
	      (no-recurse nil)
	      (t (save-excursion (up-list 1) (hargs:sexpression-p t))))))))


(ert-deftest hargs-tests--sexpression-p ()
  "Verify behavior of `hargs:sexpression-p'."
  (with-temp-buffer
    (insert " (setq var (+ 1 2))  ")
    ;; pos ->123456789012345678901
    (dolist (v '((1 nil nil)
                 (2 "(setq var (+ 1 2))" "(setq var (+ 1 2))")
                 (3 "(setq var (+ 1 2))" nil)
                 (4 "(setq var (+ 1 2))" nil)
                 (11 "(setq var (+ 1 2))" nil)
                 (12 "(+ 1 2)" "(+ 1 2)")
                 (13 "(+ 1 2)" nil)
                 (18 "(+ 1 2)" nil)
                 (19 "(+ 1 2)" "(+ 1 2)")
                 (20 "(setq var (+ 1 2))" "(setq var (+ 1 2))")
                 (21 nil nil)))
      (goto-char (car v))
      (should (string= (cadr v) (hargs:sexpression-p)))
      (should (string= (caddr v) (hargs:sexpression-p t))))))

@matsl matsl force-pushed the matsl-rsw-add-test-for-hargs_sexpression-p branch from 0d72ede to d85dd94 Compare January 20, 2024 18:45
@matsl matsl requested a review from rswgnu January 20, 2024 18:45
@matsl matsl merged commit 871391f into rsw Jan 20, 2024
@matsl matsl deleted the matsl-rsw-add-test-for-hargs_sexpression-p branch January 20, 2024 18:46
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.

None yet

2 participants