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

QA for latest changes #500

Closed
vemv opened this issue Oct 8, 2021 · 7 comments
Closed

QA for latest changes #500

vemv opened this issue Oct 8, 2021 · 7 comments

Comments

@vemv
Copy link
Member

vemv commented Oct 8, 2021

I've got a couple reports that cljr-slash is not working for the latest versions of refactor-nrepl + clj-refactor.el.

When I tried to debug with those users, the backend part seemed in order e.g. (refactor-nrepl.ns.libspecs/namespace-aliases) returns a proper map. There's a good deal of unit-testing in refactor-nrepl anyway.

I don't know so much about clj-refactor.el though. I have barely performed changes to it yet. I see in the commit log cljr-slash got bit of a revamp earlier this year.

I use an old version of clj-refactor.el and have it hard to use $latest I believe. So whenever I QA things, what I'm trying out is refactor-nrepl (official, latest) + my clj-refactor.el (old, divergent). So it's not exactly realistic QAing.

I wonder if someone in the team would have the time to give the latest refactor-nrepl + clj-refactor.el versions a go? @expez @magnars @yuhan0

🍻 - V

@vemv
Copy link
Member Author

vemv commented Oct 8, 2021

As a pereliminary diagnostic, this is part of the user error report I got (using clj-refactor.el master, latest)

Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  substring(nil 0 0)
  replace-regexp-in-string("\n" " " nil)
  cljr--keywordp(nil)
  cljr--in-keyword-sans-alias-p()
  ...

It seems to have to do with (cider-symbol-at-point). In #493 (WIP) we found out (thing-at-point 'symbol) is more desirable?

Would love if someone could just give a QAing / debugging session and weed out various unforeseen cases for this conditional:

(when-let (aliases (and cljr-magic-requires
(not (cljr--in-map-destructuring?))
(not (cljr--in-ns-above-point-p))
(not (cljr--in-reader-literal-p))
(not (cider-in-comment-p))
(not (cider-in-string-p))
(not (cljr--in-keyword-sans-alias-p))
(not (cljr--in-number-p))
(clojure-find-ns)
(cljr--magic-requires-lookup-alias)))

I'd say it's easier than it sounds!

@yuhan0
Copy link
Contributor

yuhan0 commented Oct 8, 2021

I can give the latest 3.0.0 refactor-nrepl a go next week, but from the traceback it just looks like a missing type check in cljr--in-keyword-sans-alias-p.
Both cider-symbol-at-point and thing-at-point will return nil when not in a symbol.

(defun cljr--in-keyword-sans-alias-p ()
  "Checks if thing at point is keyword without an alias."
  (let ((sym (cider-symbol-at-point)))
    (and
+        (stringp sym)
         (cljr--keywordp sym)
         (not (string-match-p "::.+" (cljr--symbol-prefix sym))))))

I wonder if this will solve it.

@vemv
Copy link
Member Author

vemv commented Oct 9, 2021

Thanks!

Sounds likely, looking forward to your session

@vemv
Copy link
Member Author

vemv commented Oct 12, 2021

👋 Any luck with the QAing?

@vemv
Copy link
Member Author

vemv commented Oct 12, 2021

Update, the error two users facing was clojure-emacs/parseclj#35 so there isn't necessarily a bug on clj-refactor/refactor-nrepl's end

@yuhan0
Copy link
Contributor

yuhan0 commented Oct 15, 2021

Yup, I've been using the latest refactor-nrepl and clj-refactor.el all week (including cljr-slash) without any issues.

@vemv
Copy link
Member Author

vemv commented Oct 15, 2021

Thanks! Between that and clojure-emacs/cider#3072 I don't fear anymore about these features working properly.

@vemv vemv closed this as completed Oct 15, 2021
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

No branches or pull requests

2 participants