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

Lread cljs proper cljs meta file fix #2

Open
wants to merge 9 commits into
base: cljs-proper
Choose a base branch
from

Conversation

martinklepsch
Copy link
Member

Includes #1 and some more work towards extracting the core bits of Codox into a reusable library that is focused on extracting plain data from Clojure and ClojureScript runtimes.

Mostly opening this to share some initial work I've done with @lread

lread and others added 9 commits April 10, 2019 19:34
For maintainability and to reduce confusion, I removed unused: html writer,
resources and project.clj. This will also avoid unused sources coming into
play for upcoming unit test work.
Updated ClojureScript's reader to use the :file metadata returned by analysis.
It formerly set :file to the actual path of the :file it was analysing. This
change matches the Clojure reader behaviour and allows for potemkin import-vars
type manipulation of metadata to, for example, point at an alternate var's docs
in a different source file.

To verify readers are still returning data that we expect, I added some tests
based on the existing playgound under test-sources. The tests are a good start
but not comprehensive and should be extended as needed. Circleci config
is included to automatically run tests on commit.

While adding tests, I noticed small bugs and made corrections:
1. The ClojureScript reader now sorts protocol methods by name. This
   matches the behaviour of the Clojure reader.
2. The Clojure reader will now add a single var for each defrecord
   with the name of the defrecord. This matches the ClojureScript
   behaviour and is apparently the desired behaviour for cljdoc.

For consistency and testability:
- we are now removing keys with empty/nil values from reader analysis maps.
- to be conistent with the ClojureScript reader, the Clojure reader no longer
  returns :file :line :column :end-column and :end-line on the namespace
  element.

Notes:

As part of this change, the ClojureScript reader will stop supressing protocol
functions when the actual source does not match the :file metadata. This allows
for a tool such as import-vars to point to, and have documented, an imported
protocol function. No action was needed for the Clojure reader to make this
work.

This change was made a bit challenging due to cljdoc's usage of codox's
root-dir and :file metadata:
- The original intent of codox is that the config :root-dir point to the
  project (aka git) root of a project and that the :file metadata
  was simply what was returned by analysis. Codox added the :path to analysis
  metadata to resolve :file to the :root-dir.
- Cljdoc does not set the :root-dir to the project root, ignores the :path and
  assumes that the :file is always relative to configured :source-path. Although
  this usage works for cljdoc, it did sprain my brain while making this change.
- To compensate, I have been careful to ensure that :file continues to always
  be relative to the the configured :source-path.

Another oddity was that altered :file metadata is being resolved to paths
within the jar file when codox is called from cljdoc. This might be due to the
way the classpath is setup when doing an analysis run. My change is coded to
handle this situation when it arises.
@lread
Copy link
Member

lread commented Aug 15, 2019

thanks @martinklepsch!

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