Consider refactoring generation of `default.md` into a single file
Opened this issue · 6 comments
Please confirm that you have searched existing issues in the repo
Yes, I have searched the existing issues
Any related issues?
What is the area that this feature belongs to?
CLI, Code Quality
Is your feature request related to a problem? Please describe.
Initially brought up during a discussion in #2278, we can consider refactoring and combining the two ways of generating default.md in the site generated by markbind init commands.
Currently, there are two ways that a site is generated: markbind init generates a MarkBind site from a given template (default template is used if none is specified), and markbind init --convert converts a pre-existing set of documents into a MarkBind site.
For the former, we simply use the default.md specified in the default template, which can be currently found in packages/core/template/default/_markbind/layouts/default.md. This behavior is dictated in packages/core/src/Site/template.ts, where we directly copy over files from the default template to the generated site.
For the latter, we use the siteConvertLayout.njk found in packages/core/src/Site/siteConvertLayout.njk, where custom values from the user's pre-existing site can be inserted into the layout. These values are given default values to fall back on if the user does not have custom values.
The core issue is that much of these two files are similar, and given that we can specify default values within siteConvertLayout.njk, it feels redundant to have the current default.md from a design perspective. An example of an inconvenience is highlighted within #2278, where some of the more recent changes (at the time of writing) were specified in default.md but not in siteConvertLayout.njk, leading to unexpectedly different behaviors for markbind init and markbind init --convert.
Describe the solution you'd like
The low-hanging fruit solution here would be to simply document the need to update both within the developer guide.
Alternatively, and a better solution from a design perspective, would be to combine these two files into a single file that dictates the behavior for a generated default.md.
Of course, this is up for discussion as we should also consider (1) whether this is more hassle than necessary if this was well-documented, and (2) whether behaviors for these two files will deviate further in the future (pointed out by @tlylt) that could necessitate having these two as separate files.
Describe alternatives you've considered
No response
Additional context
No response
+1 on the idea. This would ensure site conversion delivers the same or similar UX to initialising a new site as well.
Of course, this is up for discussion as we should also consider (1) whether this is more hassle than necessary if this was well-documented, and
The refactor should definitely be in a way that does more good than harm.
(2) whether behaviors for these two files will deviate further in the future (pointed out by @tlylt) that could necessitate having these two as separate files.
Very hard to tell but I think we can cross that bridge when we get there 🙂. We could probably do something like breaking down default.md even further as well.
On the same lines if we can share layouts with the docs folder (and anywhere else), I think it'll be a huge plus for the amount of layout files we have to maintain as well.
Hello,
It seems to me that siteConvertLayout.njk is already capable generating a default.md through the Site.convert() .
With a few changes to the behavior of Site.convert() and its helper functions, we maybe able to reuse them for something like Site.init() that would generate default.md with no need for copying of existing files.
This way may also result in the possibility of choosing templates to convert to in the future.
Hi @WillCWX, as you mentioned siteConvertLayout.njk is capable of generating a default.md when we are converting an existing site to MarkBind, i.e. using markbind init --convert and calling Site.convert() as you have pointed out.
At this juncture, I fully agree with @ang-zeyu that we can consider potential differences in behavior between markbind init and markbind init --convert only when the need arises. So yes, having siteConvertLayout.njk be the default generator for default.md makes sense.
With a few changes to the behavior of
Site.convert()and its helper functions, we maybe able to reuse them for something likeSite.init()that would generatedefault.mdwith no need for copying of existing files.
Key points to take note would be:
- Making sure that the
default.mdgenerated bysiteConvertLayout.njkis the same as the currentdefault.md - Making sure there are no unintentional changes in generated
default.mddue to nunjucks behaviour (an example is the generation of blank lines that were fixed in #2278)
This way may also result in the possibility of choosing templates to convert to in the future.
Currently we only have the default and the minimal template, but yes, that is a potential plus 👍
On the same lines if we can share layouts with the docs folder (and anywhere else), I think it'll be a huge plus for the amount of layout files we have to maintain as well.
Also, @ang-zeyu, was the reason for the d.moderate tagging because of this?
Will you be working on this issue? Can I help?
Key points to take note would be:
- Making sure that the default.md generated by siteConvertLayout.njk is the same as the current default.md
The lazy way to do this seems to be just statically place the existing default.md navigations into siteConvertLayout.njk and then make Site.buildNav() ignore these files. Though that would mean any changes to the default template's filenames would require changing both siteConvertLayout.njk and Site.buildNav(). To make it somewhat sane, we might need a file that export template navigation constants.
* [Home :house:]({{ baseUrl }}/index.html)
* [Topic 1]({{baseUrl}}/contents/topic1.html)
* [Topic 2]({{baseUrl}}/contents/topic2.html)
* Topic 3 :expanded:
* [Topic 3a]({{baseUrl}}/contents/topic3a.html)
* [Topic 3b]({{baseUrl}}/contents/topic3b.html)
The most annoying part of the generation is probably the * Topic 3 :expanded: and the [Home :house:]. Otherwise, the navigation can just be generated normally as it is now.
The even lazier/better way may be to remove both * Topic 3 :expanded: and [Home :house:] altogether
It also seems to be together/related with the #1866 issue as markbind init --convert seems to need an overhaul.
Will you be working on this issue? Can I help?
If you want to open a PR that tackles this issue, please feel free to!
The lazy way to do this seems to be just statically place the existing
default.mdnavigations intositeConvertLayout.njkand then makeSite.buildNav()ignore these files.
I assume you mean to do it in a manner similar to how siteConvertLayout.njk (and relevant parts of Site/index.ts) implements the defaultFooter? Not entirely sure what you mean here...
Though that would mean any changes to the default template's filenames would require changing both
siteConvertLayout.njkandSite.buildNav().
That should be fine - I would imagine it's a matter of documentation.
To make it somewhat sane, we might need a file that export template navigation constants.
Good idea! Site/template.ts might be a good place to store these (or a new file - we can decide later).
The most annoying part of the generation is probably the
* Topic 3 :expanded:and the[Home :house:]. Otherwise, the navigation can just be generated normally as it is now.The even lazier/better way may be to remove both
* Topic 3 :expanded:and[Home :house:]altogether
I believe we can simply set something like defaultSiteNav to conditionally generate either the default one or the custom one (in case of conversion)?