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

Support simultaneous Clojure & ClojureScript output #220

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ptaoussanis
Copy link

Fixes #216.

Live examples of the current cross-platform output:

High-level notes

  • Please see individual commits for more info.
  • I've tried to match project conventions for commit and code style.
  • Just ping if you'd like any changes.
  • No changes to README are included. Happy to add those if you're otherwise satisfied with the PR.
  • This is intended to be a purely additive PR- i.e. is intended to not break anyone using the new features.

Implementation notes

  • The changes related to cross-platform support should not affect those using {:language :clojure} or {:language :clojurescript}.
  • An optional mechanism is provided for advanced users that want to avoid broken links when upgrading a project from single platform to cross-platform.
  • This implementation does (ab)use the internal project map for conveying relevant internal state. I'm satisfied with this approach, but drawing attention to it in case you feel differently.
  • I don't personally make use of the html/write-documents code, and haven't tested it specifically - but I see no reason why this PR should cause any problems with it.

Thanks!

Author: @ptaoussanis

- This implementation attempts to be minimally invasive.
  Most of the logical changes are within `html/write-index` and
  `html/write-namespaces`, where a special path is introduced
  for cross-platform projects.

- NO behavioural changes are intended for traditional
  (non-cross-platform) projects.

- See weavejester#216 for detailed feature discussion.
Author: @ptaoussanis

This is an advanced option to help prevent any broken doc links
when upgrading a project's docs from single language to dual language.

In this case, :base-language can be set to the previous (single)
language for which doc links may already exist in the wild.

In detail, if cross-platform project then:

  {:base-language nil}            => ".clj", ".cljs" file extensions ; Default
  {:base-language :clojure}       => nil,    ".cljs" file extensions
  {:base-language :clojurescript} => ".clj", nil     file extensions

For example:

  Library Foo previously used {:language :clojure} (either because
  it was Clojure only, or because of limitations in Codox).

  Various links to Foo's Codox documentation now exist in the wild.

  Foo's authors want to change to cross-platform, but don't
  want to break pre-existing links in the wild.

  In this case, Foo's authors can use the following opts:
    {:language #{:clojure :clojurescript}
     :base-language :clojure}

  This will produce files like the following:
    com.foolib.html      ; For Clojure       platform
    com.foolib.cljs.html ; For ClojureScript platform

  Any pre-existing links will successfully point to the same
  (Clojure) docs they did previously.
Author: @ptaoussanis

Note that this commit introduces new classes to `default.css` used (only)
by "cross-platform" projects.

This means:

  - NO behavioural changes are intended for non-cross-platform projects.
  - Cross-platform projects WILL require a theme that includes the
    new CSS classes.
@weavejester
Copy link
Owner

Can you briefly go over the changes to the internal project map? I'm not sure I understand the purpose of sorting, or the :cross-platform? key, or why :var-langs needs to be its own key. My admittedly naive assumption was just to add a :language key to each namespace. Would that be insufficient information for the writer?

@ptaoussanis
Copy link
Author

Can you briefly go over the changes to the internal project map?

Sure, can do. So the general flow of changes is as follows:

1: main/generate-docs modified to use new main/cross-platform-options

This does some housekeeping on the new :language options, incl.:

  • Ensuring that :language is always a single keyword, or nil (when cross-platform).
  • Ensuring that :languages is always a set of >1 keyword (when cross-platform).

It also adds the :cross-platform? key so that downstream functions can easily identify when they're producing docs for a cross-platform project. This has some effects on what's rendered, etc. - grep for :cross-platform? to see what's affected.

As an alternative, downstream project consumers could check for a :languages set - but I prefer being explicit and not tie consumers to a potential implementation detail.

2. main/read-namespaces modified

For single-platform projects this did+does output a seq of namespaces.
For cross-platform projects this now outputs {<language> <seq-of-namespaces>}.

3. main/generate-docs modified to use new main/get-var-langs

This is to support the new block of code in html/var-docs, to generate platform labels for each var.

My admittedly naive assumption was just to add a :language key to each namespace. Would that be insufficient information for the writer?

At this point we want to know for any given var- for which platforms do we have an implementation.

The pre-existing data structures don't easily answer that question. main/get-var-langs produces the appropriate data structure, as described in its docstring.

Other stuff

All other changes are for the writer, and hopefully clear in context.

I'm not sure I understand the purpose of sorting

This just gives Codox the ability to determine the order in which different languages are displayed in menus. E.g. should the menu show "Clojure ClojureScript" or "ClojureScript Clojure"?

Without the explicit sort, the order is ~random (determined by a seq on the :languages set).

Why does order matter? I think it's more natural for Clojure to precede ClojureScript. And if additional languages are added later, we might otherwise end up with an awkward order like "ClojureScript Babashka Clojure", etc.

In case you didn't notice, please note that there's additional guidance in the individual commit messages.

Hope that helps 👍

@weavejester
Copy link
Owner

What if we ordered the languages. So the configuration would be:

{:languages [:clojure, :clojurescript]}

This would provide an explicit sort order.

We deprecate :language and have a function to automatically migrate the data over for old configurations.

(defn- convert-deprecated-language-option [options]
  (cond-> options
    (contains? options :language)
    (-> (update :languages (fnil conj []) (:language options))
        (dissoc :language))))

When it comes to generating the internal data structure, my suggestion would be to wrap the namespaces in a platform map, and do nothing more, at least not in the codox.main namespace. The reason for this is that keys like :cross-platform? and :var-langs are just indexes; i.e. they convey no information that can't be derived from the other keys.

Instead, my thought is that information would be better placed in the writer. :cross-platform? doesn't seem worth the cost, we might as well do:

(defn- cross-platform? [{:keys [languages]}]
  (> (count languages) 1))

As for var-langs, again that seems like something the writer could calculate on the fly. Iterate through all the namespaces and create the index when we need it, rather than ahead of time. After all, there's no guarantee that a particular writer will actually want that index; i.e. we could imagine a Markdown writer that might not care.

@ptaoussanis
Copy link
Author

Hi James, thanks for your thoughts on this!

Unfortunately I need to shift my attention elsewhere, so won't be able to continue on this issue. Please feel free to use anything in my PR if it's helpful, or to close it if you prefer.

Thanks again for all your assistance on this (and all your work on Codox, and other libs besides)! And again, my apologies.

@weavejester
Copy link
Owner

No problem! There's quite a bit here I can use. When I get a little time free I'll see about polishing off the implementation. Thanks for all your work on this so far.

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.

[Feature request] Support simultaneous Clojure & ClojureScript output
2 participants