Skip to content

Mark put-clojure-indent safe for use in LocalVariables, .dir-locals.el, etc. #598

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

Merged

Conversation

rlbdv
Copy link
Contributor

@rlbdv rlbdv commented Jul 3, 2021

Add a put-clojure-indent form validator and attach it as a
safe-local-eval-function property so that safe invocations won't
trigger open-file prompts when used in local variables, or when added
to .dir-locals.el like this:

  ((clojure-mode
    (eval . (put-clojure-indent 'defrecord '(2 :form :form (1))))))

For now, only support specs specified as lists, not vectors.


This work assumes that we don't want to rely on any existing validation, and so attempts to perform thorough validation up front. Also happy to make further changes, but thought I'd post what I have, to see if I'm on a reasonable track.

@rlbdv
Copy link
Contributor Author

rlbdv commented Jul 3, 2021

I don't have access to circleci at the moment (wondering why their test results aren't public without a login), but since the test and test-checks targets work here, I wondered if it might be the test-bytecomp target. That one fails on the bare $ in the target rules, and if I do this:

--- a/Makefile
+++ b/Makefile
@@ -38,5 +38,5 @@ test-checks:
 test-bytecomp: $(SRCS:.el=.elc-test)

 %.elc-test: %.el elpa
-       $(CASK) exec $(EMACS) --no-site-file --no-site-lisp --batch \
-               -l test/clojure-mode-bytecomp-warnings.el $
+       $(CASK) exec $(EMACS) -L . --no-site-file --no-site-lisp --batch \
+               -l test/clojure-mode-bytecomp-warnings.el $<

It gets further, but fails like this:

In toplevel form:
clojure-mode.el:482:25:Error: reference to free variable `clojure--let-regexp'

After moving some functions around and removing the #s from (make-obsolete #'clojure-no-space-after-tag #'clojure-space-for-delimiter-p "5.12.0"), it gets further still, but then crashes because end-of-thing is undefined. I'm just guessing that might indicate a missing thingatpt dependency.

@rlbdv rlbdv force-pushed the validate-put-clojure-indent-as-safe branch from 5baa06e to 99d3de9 Compare July 23, 2021 07:17
@rlbdv
Copy link
Contributor Author

rlbdv commented Jul 23, 2021

Not sure what's going on in CI with 25 and master, but I went ahead and fixed up everything that was causing trouble with Emacs 27 here.

@rlbdv
Copy link
Contributor Author

rlbdv commented Jul 23, 2021

i.e. I moved those declarations before their first use, and added the missing (require 'thingatpt), etc.

@bbatsov
Copy link
Member

bbatsov commented Jul 23, 2021

Your changes look good to me overall, but I won't be able to review them more carefully until next week. Sorry about the slow response.

@rlbdv
Copy link
Contributor Author

rlbdv commented Jul 23, 2021

No worries, and no rush -- if I get time, I might see what's up with emacs 25 and master later, either locally, or in a vm.

@rlbdv rlbdv force-pushed the validate-put-clojure-indent-as-safe branch from 99d3de9 to c442aa5 Compare July 24, 2021 19:16
@rlbdv
Copy link
Contributor Author

rlbdv commented Jul 24, 2021

Fixed the issues. Emacs 25 doesn't have caddr (switched to nth which is likely more approachable anyway), and test-bytecomp wouldn't allow the existing clojure-align-separator docstring because it's longer than 80 characters.

rlbdv added 8 commits August 20, 2021 17:57
Add a put-clojure-indent form validator and attach it as a
safe-local-eval-function property so that safe invocations won't
trigger open-file prompts when used in local variables, or when added
to .dir-locals.el like this:

  ((clojure-mode
    (eval . (put-clojure-indent 'defrecord '(2 :form :form (1))))))

For now, only support specs specified as lists, not vectors.
Otherwise test-bytecomp fails with:

  In toplevel form:
  clojure-mode.el:493:25:Error: reference to free variable `clojure--let-regexp'
Otherwise test-bytecomp fails like this:

  In toplevel form:
  clojure-mode.el:1860:23:Error: assignment to free variable `clojure-cached-ns'
Otherwise test-bytecomp fails like this:

  In toplevel form:
  clojure-mode.el:3015:1:Error: the function `clojure-no-space-after-tag' is not known to be defined.
In toplevel form:
clojure-mode.el:1135:1: Error: custom-declare-variable `clojure-align-separator' docstring wider than 80 characters
make: *** [Makefile:41: clojure-mode.elc-test] Error 1
@rlbdv rlbdv force-pushed the validate-put-clojure-indent-as-safe branch from 4af9825 to 58836df Compare August 20, 2021 22:58
@bbatsov bbatsov merged commit e1dc7ca into clojure-emacs:master Aug 21, 2021
@bbatsov
Copy link
Member

bbatsov commented Aug 21, 2021

Well done!

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