clojure-emacs / clj-refactor.el

A CIDER extension that provides powerful commands for refactoring Clojure code.
GNU General Public License v3.0
771 stars 111 forks source link

`cljr-rename-symbol` not working properly when narrowing is in effect #537

Closed rechvs closed 1 year ago

rechvs commented 1 year ago

Expected behavior

When executing cljr-rename-symbol on a function name while narrowing is in effect, cljr-rename-symbol should either execute the renaming as if no narrowing was in effect or issue a message about being unable to do so.
I would of course prefer the first option, but I understand that this might be impossible, since the narrowing takes away context necessary for a thorough renaming in the whole file. Maybe a compromise would be to only execute the renaming in the narrowed buffer content? It would then be up to the user to ensure that the narrowed content contains all context relevant for the renaming.

Actual behavior

cljr-rename-symbol claims (via a message in the echo area) to have renamed the function without having actually done that.

Side note: when trying to rename function arguments while narrowing is in effect, the message cljr-rename-symbol: Wrong type argument: char-or-string-p, nil is issued (and the argument isn’t actually renamed).

Steps to reproduce the problem

  1. Assuming that you have a running CIDER session, add the following function definition to a Clojure buffer:
    (defn some-function [some-arg]
    (+ 1 2 3))
  2. Narrow the buffer to the function definition by placing point within it and executing narrow-to-defun.
  3. Place point on the function name, execute cljr-rename-symbol, answer y on the first prompt, provide a new name on the second prompt and press Enter.
  4. The message Renamed 1 occurrences of some-function appears in the echo area, but the function name in the buffer hasn’t actually changed.

Environment & Version information

clj-refactor.el version information

clj-refactor 3.6.0 (package: 20221023.1644), refactor-nrepl 3.6.0

CIDER version information

CIDER 1.6.0 (Buenos Aires), nREPL 1.0.0
Clojure 1.10.1, Java 11.0.8

Leiningen version

2.10.0 on Java 11.0.8 OpenJDK 64-Bit Server VM

Emacs version

GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0) of 2021-03-28, modified by Debian

Operating system

Linux 4.19.0-6-amd64 Debian 4.19.67-2+deb10u2 (2019-11-11) x86_64 GNU/Linux

vemv commented 1 year ago

Hi!

Thanks for the report. I'd accept a PR that failed-fast the feature if buffer-narrowed-p was truthy.

Other than that I don't foresee a lot of people intentully using cljr-rename-symbol while narrowing. I also don't have the time to design sensible semantics for it, etc.

rechvs commented 1 year ago

I'd accept a PR that failed-fast the feature if buffer-narrowed-p was truthy.

My pleasure, here you go.

Other than that I don't foresee a lot of people intentully using cljr-rename-symbol while narrowing. I also don't have the time to design sensible semantics for it, etc.

I understand. I guess I should start thinking about my workflow, if it involves all to much narrowing... :sweat_smile: