Also consider cljc files for namespaces-on-classpath

[eval]

Fixes #90

[eval]

Because namespaces-on-classpath only returns the namespace name, it's safe (not dependent on reader conditionals) to return results for clj, cljc, and cljs alike.

I quickly scanned clj-suitable. Do I understand your suggestion correctly in that we'd add (.endsWith file ".cljs") to namespaces-on-classpath, label the candidates in some way and then adjust compliment.sources.namespaces-and-classes to only filter out the clj(c) namespaces based on that labeling?

It would seem desirable to craft a test for namespaces-on-classpath ensuring that all 3 kinds of results are returned. I could contribute a commit if you happen to be in a rush.

🙏🏻

[vemv]

Currently, the following is returned:

{:candidate ns-str, :type :namespace}

...it would seem easy enough to make it also include :extension ".cljc"? Depending on the actual filename.

(Note that I chose the word :extension, not :context, :platform, etc. This makes the notion very easy to define and does not force us to think of other semantics for now. Which can be added later)

Then, consumers like clj-suitable could remove entries where the :type was :namespace and the :extension was ".clj" i.e. not .cljs or .cljc.

[eval]

Adjusted to allow for getting cljs namespaces out as well. Fully bw-compatible.
Let me know WYT!

[eval]

@vemv added tests as that turned out easier than expected. Hope that didn't cross any work you already did.

[alexander-yakushev]

Thank you for the PR! Since the scope of the fix has grown, let me grump and bikeshed a little bit.

Maybe this is irrational, but computing and attaching this :extension key to the results rubs me the wrong way. Even though a file extension is a clear-cut concept, I feel like it's a wrong entity to introduce on the Compliment side. I think it is better to attach the whole :file to go together with completions for namespaces. This in theory gives more choice to upstream users of Compliment (however few there are) to filter namespace completions based on the extension but also anything else (file on disk vs inside JAR? I don't know).

I'm also not a big fan of :name-only false approach. I've done exactly the same for candidates before with :tag-candidates true, and it just doesn't feel very clean. Since namespaces-on-classpath is an internal function (but still public), it's better to introduce a separate function, e.g. namespaces+files-on-classpath, with the new functionality. The old function can be deprecated and then removed sometime in the future.

Let me know if any of this sounds dumb.

[eval]

I think it is better to attach the whole :file to go together with completions for namespaces.

Makes sense!

it's better to introduce a separate function, e.g. namespaces+files-on-classpath, with the new functionality.

👍🏻 Should we keep the extensions-option in this new function to provide filtering for callers?

[alexander-yakushev]

Should we keep the extensions-option in this new function to provide filtering for callers?

I'd rather keep it at the callsite, to be honest. Especially since the new function doesn't return extensions anymore, it shouldn't look at them either. Just return the files and let the caller filter the list.

[eval]

@alexander-yakushev thanks for the comments! Changes applied.

[vemv]

fwiw, LGTM!

[alexander-yakushev]

Looks perfect now, thank you for following through!