Enable cljr-slash-uses-suggest-libspec by default

[dgtized]

Also mark functions and defcustoms that will deprecate once suggest-libspec is the only path available, and expanded the documentation on the feature flag to provide context.

This is a step towards deprecating the old cljr-slash behavior provided by the namespace-aliases middleware started in #531 and implemented in #532. We have had this as an optional feature flag for over a year in total, so now we are trying to transition folks over to make it the default. Presuming no issues, we can then deprecate support for the old middleware.

As #530 is not complete, it's still not possible to specify language context in magic requires. However, even without that functionality, this implementation is a marked improvement over the old two stage prompt for language context, and then prompt for candidate requires. #530 was addressed by clojure-emacs/refactor-nrepl#392.

In the process of looking for future deprecations I also noted that cljr-suggest-namespace-aliases no longer appears to be an option we pass to the middleware. I believe we do this by default, but might be worth double checking before fully deprecating.

---

Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

• The commits are consistent with our contribution guidelines
• You've added tests (if possible) to cover your change(s)
• The new code is not generating byte compile warnings (run cask exec emacs -batch -Q -L . -eval "(progn (setq byte-compile-error-on-warn t) (batch-byte-compile))" clj-refactor.el)
• All tests are passing (run ./run-tests.sh)
• You've updated the changelog (if adding/changing user-visible functionality)
• You've updated the readme (if adding/changing user-visible functionality)

Thanks!

[vemv]

As #530 is not complete

What's missing?

Note that https://github.com/clojure-emacs/refactor-nrepl/blob/7cc8365fa910e74e4c6d2d85f4d3d11f8c7e6618/src/refactor_nrepl/ns/suggest_libspecs.clj#L17 parses the format laid out in #530 (comment)

Have you given it a try? After that, possibly update docstrings/etc and we'd close #530

In the process of looking for future deprecations I also noted that cljr-suggest-namespace-aliases no longer appears to be an option we pass to the middleware.

The related middleware option is named "suggest" and is correctly passed by the client and handled by the middleware

Cheers - V

[dgtized]

If #530 is done then I guess we can close it or mark it as pending documentation. This has been going on for long enough I've forgotten the context. Is suggest_libspecs_test.clj the tests for that? I think the only reason this change would get blocked by that is if the new format parser is not backward compatible, but I think it allows for both the old and new. I think we can continue to update docs as need be, but I think it's most important we start getting user feedback with the new middleware and then respond to any changes there and continue to iterate on documentation. My assumption is that you and I are the only ones currently using the new middleware which seems very subpar for user testing.

Re cljr-suggest-namespace-aliases, happy to hear that is handled I just noticed it looked dead with the flag enabled.

[vemv]

Is suggest_libspecs_test.clj the tests for that? I

Yes

, but I think it allows for both the old and new.

Yes, definitely

If #530 is done then I guess we can close it or mark it as pending documentation. This has been going on for long enough I've forgotten the context.

I'd appreciate if at least one person other than me verified this to work. Also, our normal standard is to document first, release second.

If you absolutely didn't have the time I'd understand. I'll leave the PR open for a few days to leave a reasonable headroom.

Cheers - V

[dgtized]

We can keep improving the docs, there are a lot of shortfalls for both refactor-nrepl and clj-refactor.el, and I don't think this particular PR should be responsible for filling that back in. We know the suggest middleware is handling the current input format correctly so it won't break for existing users when we enable the flag, and then we can start moving folks over to other new features as we document them and do more verification then.

The reason I had forgotten that #530 was completed is there was no link between clojure-emacs/refactor-nrepl#392 to #530 when it was done, and then there was no corresponding update to the documentation here or the tests. I don't think that was assigned to anyone. For that matter, only namespace-aliases is documented there but the documentation for suggest appears to be missing entirely? I don't think that was released prematurely without documentation, but it would be beneficial to add it there.

I believe https://github.com/clojure-emacs/clj-refactor.el/wiki#magic-requires is the only non-code documentation of this feature from the clj-refactor.el side. It doesn't reference the new magic requires syntax, but is otherwise mostly accurate. I can update the reference to the feature flag once this PR has merged. In the future, I can also take a look at referencing or summarizing updated docs for the suggest middleware in refactor-nrepl from clj-refactor.el side or start incrementally distilling our various discussions on all of these PRs and issues. However, this is the time and scope I have to contribute for this week, baring any technical concerns of this PR that need fixing.

Follow ups that this feature still need in the long term, so either of us can contribute when we have time:

• Document the suggest endpoint in the refactor-nrepl middleware
• Document Ability to specify context in cljr-magic-require-namespaces #530 cljr-magic-requires-namespaces syntax both in the README/wiki and on the defcustom.
• Migrate to new end to end specs from the existing feature tests for cljr-slash as all the existing ones make expectations about the old behavior (and to continue deprecation of ecukes for behavior specs)
• Improve documentation on cljr-slash as a whole (ie actually migrate all the feature discussion from all the corresponding issues into documentation of how it currently works).
  • Document that cljr-slash handles clj, cljs, cljc language contexts
• Finish removing the deprecated functions for the functionality before cljr-slash-uses-suggest-libspec

I'm trying to incrementally move this forward, so it does not languish. I appreciate that fully documenting this feature is valuable, but I think getting it in the hands of users is most important. My hope is that by enabling this feature it will also help triage friction points that most need documentation.

[vemv]

Thanks much for laying out what's next.

I'll merge, and cut a stable+announced release a few days later as I have the chance to carefully consider our observations.

[dgtized]

I updated the wiki for https://github.com/clojure-emacs/clj-refactor.el/wiki#magic-requires to focus more on cljr-slash and the capabilities the default suggest-libspec middleware provide. It still needs further details on the custom syntax for cljr-magic-require-namespaces but now it's at least mentioning language contexts.

[vemv]

Thank you!