search

vernak2539 / astro-rehype-relative-markdown-links

Rehype Plugin for Astro that allows for usage of relative links between markdown files
https://www.npmjs.com/package/astro-rehype-relative-markdown-links
15 stars 4 forks source link

fix: remove contentPath, add srcDir & align `collection base` on a per collection basis #49

Open techfg opened 7 months ago

techfg commented 7 months ago

IMPORTANT - This PR contains the fixes in #55 so should be merged after it.

❗ IMPORTANT: Breaking Change❗

If I'm understanding the way github-changelog-generator works, adding a label named Breaking changes: to this PR and/or issue #47 should annotate the generated changelog with the information about the breaking change. Alternatively, it looks like adding a label breaking to issue #47 would do the same thing so either way "should" work.

  1. Eliminates contentPath option
  2. Adds srcDir option
  3. Eliminates contentPathMode
  4. Adds collectionBase and collections options to align the application of collection base on a per collection basis rather than unilaterally to all collections

As detailed in #47, PR #2 had some shortcomings that PR #18 did not fully address. This PR addresses the remaining issues and creates the options needed to support future flexibility (e.g., #24 & #27).

FYI - I couldn't quite get the docs to emit inline the way I wanted so still working through that part. I did add a @see section and generated a separate interface doc for the new CollectionConfig options so the docs are covered, just not the way that I would ideally want them to generate. Might be a limitation with typedoc & zod or (hopefully) just operator error. I'll keep at it and submit separate PR if you agree with this one and it gets merged beforehand.

Resolves #47

vernak2539 commented 6 months ago

I really like this idea, as it does open a lot of possibilities. I'm going to have to take a moment to load all the context back into my mind, so it may take me a few days! (i'm looking though!)

techfg commented 6 months ago

Sounds good! PR #50 which addresses Option 1 from #24 is based on this PR so this one should come first. This one also opens up the idea proposed in 53 which would build on both 50 & 24.

Ahead of this one though, would recommend you review 55, 36, 48, 51, 56 & 58 in that order first. They are all pretty small and built directly on top of main and should simplify merging for this one, 50 and the potentially 52 (review this one last and see all my comments in the PR regarding my current thoughts on it).

vernak2539 commented 6 months ago
techfg commented 18 hours ago

Hi @vernak2539 -

I merged latest from main into this PR to resolve the conflicts and hopefully makes things a bit easier on you assuming you move forward with merging this PR.

Note that the merge was not trivial due to the changes in this PR and changes in the other PRs that were merged back in May as well as recently. I believe I merged properly but given the nature of all the changes across the PRs, there is a chance that I missed something and/or merged incorrectly. All the tests pass and things should be good but just mentioning this in case any issues crop up (assuming this PR is merged) so we can circle back to this merge as the likely culprit.