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

[stacktrace] Repatriate stacktrace analyzer from Haystack #322

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

alexander-yakushev
Copy link
Member

@alexander-yakushev alexander-yakushev commented Mar 7, 2025

This PR brings haystack.analyzer back home to Orchard. This is not an exact copy; I also did a lot of work in rewriting it to make leaner and faster.

  • Pre-1.10 support bits are removed.
  • Complete rework of :project flag for frames. Previously, it was implemented by scanning the classpath for all loaded namespaces, and comparing frame's namespace against that set. Now, frame namespace tries to be resolved to classpath resource. If successful, and the path starts with project's workdir, the frame is considered :project. Ridiculously faster this way (< 1 ms vs ~100 ms in a big project).
  • Many functions were refactored without changing the behavior.
  • Test namespace was ported almost without behavior changes (but was rewritten to matcher-combinators).

The next step will be to switch cider-nrepl to this. I suspect that as I start working on that, I'll have to make some fixes here too; but I think it's fine to review and merge the current state first.


@alexander-yakushev alexander-yakushev force-pushed the stacktrace-analyzer branch 2 times, most recently from b06d34f to 9967858 Compare March 7, 2025 21:12
@@ -0,0 +1,318 @@
(ns orchard.stacktrace
"Cause and stacktrace analysis for exceptions"
{:added "0.31"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can expand this a bit, to be more in line with the level of high-level descriptions that most other namespaces have.

(is (not (:data (first causes2))))))

(deftest compilation-errors-test
(is (match? #"Syntax error compiling at \(.*orchard/stacktrace_test\.clj:"
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to have all assertions in testing, so that the output is a bit more descriptive, but it's not a big deal.

@bbatsov
Copy link
Member

bbatsov commented Mar 8, 2025

Looks pretty good to me overall.

@alexander-yakushev alexander-yakushev force-pushed the stacktrace-analyzer branch 2 times, most recently from 3580b84 to 39f5083 Compare March 8, 2025 13:21
@alexander-yakushev
Copy link
Member Author

Done.

@bbatsov bbatsov merged commit 2cecd31 into master Mar 10, 2025
20 checks passed
@bbatsov
Copy link
Member

bbatsov commented Mar 10, 2025

Great work!

@alexander-yakushev alexander-yakushev deleted the stacktrace-analyzer branch March 11, 2025 17:51
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.

2 participants