Skip to content

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

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

Closed
rechvs opened this issue Jan 13, 2023 · 2 comments
Closed

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

rechvs opened this issue Jan 13, 2023 · 2 comments

Comments

@rechvs
Copy link
Contributor

rechvs commented Jan 13, 2023

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))
  1. Narrow the buffer to the function definition by placing point within it and executing narrow-to-defun.
  2. 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.
  3. 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
Copy link
Member

vemv commented Jan 13, 2023

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 pushed a commit to rechvs/clj-refactor.el that referenced this issue Jan 17, 2023
… is in effect

Previously the function would report a successful renaming without
actually having renamed anything.
@rechvs
Copy link
Contributor Author

rechvs commented Jan 17, 2023

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... 😅

rechvs pushed a commit to rechvs/clj-refactor.el that referenced this issue Jan 20, 2023
Otherwise the linter will complain about the "if" missing its "else"
branch.
rechvs pushed a commit to rechvs/clj-refactor.el that referenced this issue Feb 21, 2023
… is in effect

Previously the function would report a successful renaming without
actually having renamed anything.
rechvs pushed a commit to rechvs/clj-refactor.el that referenced this issue Feb 21, 2023
Otherwise the linter will complain about the "if" missing its "else"
branch.
@vemv vemv closed this as completed in c63dfa9 Jul 4, 2023
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