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

Fix the debugger in case of a local shadows a var #847

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

darkleaf
Copy link
Contributor

@darkleaf darkleaf commented Feb 4, 2024

Fixes #846

Before submitting a PR make sure the following things have been done:

  • The commits are consistent with our contribution guidelines
  • You've added tests to cover your change(s)
  • All tests are passing
  • You've updated the README
  • Middleware documentation is up to date
    • Please check out and modify the cider.nrepl ns which has all middleware documentation.
    • Run lein docs afterwards, and commit the results.

Note: If you're just starting out to hack on cider-nrepl you might find
nREPL's documentation and the
"Design" section of the README extremely useful.*

Thanks!


  1. resolve have an extra arity for &env
  2. I added arity for looks-step-innable? for backwards compatibility
(defn ns-resolve
  "Returns the var or Class to which a symbol will be resolved in the
  namespace (unless found in the environment), else nil.  Note that
  if the symbol is fully qualified, the var/Class to which it resolves
  need not be present in the namespace."
  {:added "1.0"
   :static true}
  ([ns sym]
    (ns-resolve ns nil sym))
  ([ns env sym]
    (when-not (contains? env sym)
      (clojure.lang.Compiler/maybeResolveIn (the-ns ns) sym))))

(defn resolve
  "same as (ns-resolve *ns* symbol) or (ns-resolve *ns* &env symbol)"
  {:added "1.0"
   :static true}
  ([sym] (ns-resolve *ns* sym))
  ([env sym] (ns-resolve *ns* env sym)))

@darkleaf darkleaf changed the title Fix the debugger in case of a local shadows a var. refs #846 Fix the debugger in case of a local shadows a var Feb 4, 2024
@vemv
Copy link
Member

vemv commented Feb 4, 2024

Nice fix, thanks much!

I will merge it. First, please:

  • satisfy the small suggestion
  • satisfy the PR checklist
  • add an entry to the CHANGELOG
  • run export LEIN_JVM_OPTS="-Dmranderson.internal.no-parallelism=true"; PROJECT_VERSION=0.45.0 make install in the project root
    • make sure 0.45.0 matches with the version you normally use
    • use this locally-installed version for a reasonable period, so that we can gain confidence in that nothing in the debugger broke.

Cheers - V

@darkleaf
Copy link
Contributor Author

darkleaf commented Feb 4, 2024

Снимок экрана 2024-02-04 в 22 07 40

@@ -570,9 +572,8 @@ this map (identified by a key), and will `dissoc` it afterwards."}
`irrelevant-return-value-forms`."
[&env form]
(or (and (symbol? form)
(not (contains? &env form))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not (contains? &env form)) is handled inside (resolve &env form)

@bbatsov
Copy link
Member

bbatsov commented Feb 19, 2024

The changes look good to me. @vemv did you by any chance forgot to revisit this PR?

CHANGELOG.md Outdated Show resolved Hide resolved
@vemv
Copy link
Member

vemv commented Feb 20, 2024

@darkleaf have you been testing out this changes locally in the meantime?

@darkleaf
Copy link
Contributor Author

darkleaf commented Feb 20, 2024

@darkleaf have you been testing out this changes locally in the meantime?

Yes

@vemv
Copy link
Member

vemv commented Feb 20, 2024

Awesome - thank you!

PR LGTM after addressing @bbatsov 's feedback

CHANGELOG.md Outdated
@@ -2,6 +2,10 @@

## master (unreleased)

## Bugs fixed

* `middleware.debug`: use `resolve` with `&env` to fix debugging in case of a local shadows a var.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `middleware.debug`: use `resolve` with `&env` to fix debugging in case of a local shadows a var.
* [#846](https://github.com/clojure-emacs/cider-nrepl/issues/846): `middleware.debug`: use `resolve` with `&env`, preventing exceptions if a local shadows a var.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added link to the PR. Thanks.
But there were no exceptions, just a local shadowing.

@vemv vemv merged commit 3824d72 into clojure-emacs:master Feb 21, 2024
63 checks passed
@vemv
Copy link
Member

vemv commented Feb 21, 2024

Thanks much!

There should be a release within the next few days.

@darkleaf darkleaf deleted the local-shadows-var branch February 21, 2024 13:53
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.

Name collision causes an exception during debugging
3 participants