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

cljr-rename-symbol doesn't play well with 'pretty-mode' #429

Open
JAremko opened this issue Aug 18, 2018 · 19 comments
Open

cljr-rename-symbol doesn't play well with 'pretty-mode' #429

JAremko opened this issue Aug 18, 2018 · 19 comments

Comments

@JAremko
Copy link

JAremko commented Aug 18, 2018

Steps to reproduce the problem

Run cljr-rename-symbol with foo renamed to bar on

(defn foo [arg1 arg2])
(foo 1 2)
(partial foo 1)

Expected behavior

(defn bar [arg1 arg2])
(bar 1 2)
(partial bar 1)

Actual behavior

(defn bar [arg1 arg2])
(bar 1 2)
(partial foo 1)

Environment & Version information

CIDER version

;; CIDER 0.18.0snapshot (package: 20180802.1919), nREPL 0.2.13
;; Clojure 1.9.0, Java 10.0.1

clj-refactor.el version

2.4.0-SNAPSHOT

Lein version

Leiningen 2.8.1 on Java 10.0.1 OpenJDK 64-Bit Server VM

Emacs version

26.1

@expez
Copy link
Member

expez commented Aug 18, 2018

Can't reproduce. From lein new test I got:

(ns test.core)

(defn foo
  "I don't do a whole lot."
  [x]
  (println x "Hello, World!"))

(foo 1)

(partial foo 1)

Rename foo to bar:

(ns test.core)

(defn bar
  "I don't do a whole lot."
  [x]
  (println x "Hello, World!"))

(bar 1)

(partial bar 1)

You already had a bar and wanted foo to have the same name or something? Need more details to reproduce.

@JAremko
Copy link
Author

JAremko commented Aug 18, 2018

@expez bad copy-pasta in the first listing. fixed.

@JAremko
Copy link
Author

JAremko commented Aug 18, 2018

This is what I'm getting
https://i.imgur.com/oXPZD7z.png
https://i.imgur.com/xBNvQhC.png

@expez
Copy link
Member

expez commented Aug 18, 2018

Do you get the same if you load the code into the REPL before doing the refactoring?

E.g. by using C-c C-k in CIDER?

@benedekfazekas
Copy link
Member

benedekfazekas commented Aug 19, 2018 via email

@JAremko
Copy link
Author

JAremko commented Aug 19, 2018

@expez Doesn't help. Btw it reports Renamed 3 occurrences of foo so it sees all 3 occurrences.
Also sometime (after rename) I get:

@benedekfazekas OMG 🙀 it helped. I don't understand.

@JAremko
Copy link
Author

JAremko commented Aug 19, 2018

So basically cljr-rename-symbol conflicts with clojure-enable-fancify-symbols in Spacemacs. I'm just not sure how font lock shenanigans can affect cljr-rename-symbol.

@JAremko
Copy link
Author

JAremko commented Aug 19, 2018

it might confuse the column counts cljr works

@benedekfazekas Checked. You are right. It affects column number. It's strange that there was no error report or warning of any kind. Can be really nasty in big projects.


Now we just need to decide who's bug it is 😄
I vote for clj-refactor.el bug!

@benedekfazekas
Copy link
Member

benedekfazekas commented Aug 19, 2018 via email

@expez
Copy link
Member

expez commented Aug 19, 2018

Is there a way to get the 'real' column number? If not, then I don't see how cljr could handle this...

@JAremko
Copy link
Author

JAremko commented Aug 19, 2018

@expez Indirect buffers probably can be used for this. They can have different modes then the original one.

@JAremko
Copy link
Author

JAremko commented Aug 19, 2018

@expez Also in this case composite.el has function find-composition that probably can help.

@expez expez changed the title cljr-rename-symbol doesn't rename partials cljr-rename-symbol doesn't handle play well with 'pretty-mode' Aug 19, 2018
@expez expez changed the title cljr-rename-symbol doesn't handle play well with 'pretty-mode' cljr-rename-symbol doesn't play well with 'pretty-mode' Aug 19, 2018
@JAremko
Copy link
Author

JAremko commented Aug 19, 2018

Keep in mind that this also doesn't work:

(defn foo
  "I don't do a whole lot."
  [x]
  (println x "Hello, World!"))

(foo 1)

(partial map identity) (foo 1)

I don't know how clj-refactor.el actually does substitution but from what i gather the best way to solve it will be letf-ing prettyfication list around the substitution logic so nothing will be "prettyfied" when you visit files for substitution. To deal with files that already have visiting buffer you can create Indirect buffers of those without pretty-mode and substitute in them.

@expez
Copy link
Member

expez commented Aug 20, 2018

@JAremko We can get issues at both ends of this:

  1. At the beginning we send down line/col to the middleware to find occurrences of a symbol to work on .
  2. After the middleware returns we use line/col to make edits to the right locations.

Both of these will report the wrong column number with pretty-mode, but sometimes it isn't fatal (because you're still inside the same symbol).

@benedekfazekas
Copy link
Member

benedekfazekas commented Aug 20, 2018 via email

@expez
Copy link
Member

expez commented Aug 20, 2018

no way that will know about prettify symbols

Thankfully it doesn't have to, because the pretty symbols only really exist in emacs. The regular text is what's stored in the file on disk.

@benedekfazekas
Copy link
Member

benedekfazekas commented Aug 20, 2018 via email

@Nimamoh
Copy link

Nimamoh commented Mar 31, 2019

has anyone found a workaround for this issue?

@JAremko
Copy link
Author

JAremko commented Apr 1, 2019

@Nimamoh I ended up simply disabling fancy symbols primary because they also mess up long line highlight.

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

4 participants