Remove .mbdf and .mbd #1762

ong6 · 2022-02-12T16:30:16Z

What is the purpose of this pull request?

Fixes #1672

Overview of changes:

Removes allowing of .mbdf and .mbd as file extension types.

Added new section in UG documentation under reusing contents called Creating Custom Fragments, view here
Updated all related core and cli files using .mbdf and .mbd
Updated all existing documentation referring to .mbdf and .mbd.
Updated existing test cases to remove .mbdf and .mbd
Added new test for custom-fragment.md
Fixed typos/errors in documentation found while updating docs (may be unrelated change)

Anything you'd like to highlight / discuss:

Some context for this PR:

.mbd was created with the intent of making our own extension called .markbind
.mbdf was created to easily exclude fragments from page generation
Since the addition of pagesExclude with PR Add site.json properties for file exclusions #1328, the definition of fragments can be described by the users hence rendering the first 2 points obsolete.

Testing instructions:

Run npm run test in packages/cli
All test cases should pass

Proposed commit message: (wrap lines at 72 characters)

Remove .mbdf and .mbd as markbind supported file extensions.

Syntax parsers do not work with .mbdf and .mbd and
this functionality could be implemented using other means (e.g. pagesExclude)

Refer to this for the full disscussion #1672.

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

…to remove-mbd/mbdf

damithc · 2022-02-12T17:05:56Z

As this is a breaking change, we won't be able to do any v3.* releases (unless as hot patches to previous release) after merging this PR, right?

ong6 · 2022-02-12T17:41:51Z

@damithc Actually I'm not too sure about this maybe @ang-zeyu can respond?

I think its good SWE practice to release this as a new version though? Since this will indeed remove all support for existing .mbdf and .mbd files.

Perhaps I can include a new markbind command to fix all the old .mbdf and .mbd files? Thoughts on this?

…move-mbd/mbdf

damithc · 2022-02-13T07:43:54Z

@ong6 at the same time, can you insert a tip somewhere in the user docs to guide authors on how to exclude fragment files from rendering?

jonahtanjz · 2022-02-13T12:29:36Z

As this is a breaking change, we won't be able to do any v3.* releases (unless as hot patches to previous release) after merging this PR, right?

Yup. Will need to create a separate branch for hot patches releases (if any) after this is merged.

Co-authored-by: Liu YongLiang <41845017+tlylt@users.noreply.github.com>

ong6 · 2022-02-14T01:53:51Z

@ong6 Good work! I have highlighted some minor issues in the user guide. Since this PR is a breaking change, perhaps we can have a more detailed proposed commit message to explain a little bit more about the context?

@tlylt @jovyntls
Thanks spotting the errors! I've made the changes, perhaps instead of a more detailed explanation in the commit message, I will update the highlight/discussion part instead?

…move-mbd/mbdf

jonahtanjz

Nice work @ong6!

Thanks @tlylt and @jovyntls for the reviews 👍

Since this PR is a breaking change, perhaps we can have a more detailed proposed commit message to explain a little bit more about the context?

I've made the changes, perhaps instead of a more detailed explanation in the commit message, I will update the highlight/discussion part instead?

I think it's better to put it in the commit message as it will be easier to track from the commit history.

packages/cli/index.js

packages/core/src/utils/fsUtil.js

packages/cli/test/functional/test_site/sub_site/nested_sub_site/site.json

...i/test/functional/test_site/expected/sub_site/nested_sub_site/testNunjucksPathResolving.html

Co-authored-by: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com>

…to remove-mbd/mbdf

This reverts commit 1301b51.

ong6 · 2022-02-15T17:37:36Z

Nice work @ong6!

Thanks @tlylt and @jovyntls for the reviews 👍

Since this PR is a breaking change, perhaps we can have a more detailed proposed commit message to explain a little bit more about the context?

I've made the changes, perhaps instead of a more detailed explanation in the commit message, I will update the highlight/discussion part instead?

I think it's better to put it in the commit message as it will be easier to track from the commit history.

Thanks for the review @jonahtanjz I've made all the suggested changes!

jonahtanjz

Thanks for making the changes @ong6

Just 2 more comments :)

...s/cli/test/functional/test_site/sub_site/nested_sub_site/testNunjucksPathResolvingInclude.md

jonahtanjz · 2022-02-16T03:39:49Z

packages/core/src/utils/fsUtil.js

 const ensurePosix = require('ensure-posix-path');

-const markdownFileExts = ['.md', '.mbd', '.mbdf'];
+const markdownFileExts = '.md';


Will have to update isMarkdownFileExt method as well since it is no longer an array. Can use === instead of includes.

Includes will still work with strings, so there should be no issue with this particular case. Unless the change is for readability reasons?

I believe this will give false positives? Although unlikely, but files with extension .m or having no extension will be returned as true.

Ahh ok I see, will implement the change then!

…e/testNunjucksPathResolvingInclude.md Co-authored-by: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com>

…move-mbd/mbdf

…to remove-mbd/mbdf

jonahtanjz

Thanks @ong6 :) You may want to update the commit message as well following this template.

Other than that the rest looks good 👍

packages/cli/index.js

Co-authored-by: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com>

ong6 · 2022-02-16T14:59:19Z

Thanks @ong6 :) You may want to update the commit message as well following this template.

Other than that the rest looks good 👍

Oh nice spot, didn't realize that issue also! Thanks for checking my PR once again. I have updated the commit message as well!

jonahtanjz

Thanks @ong6 :)

LGTM 👍

ong6 added 12 commits August 29, 2021 16:46

Pulled from Master

55572b9

Merge branch 'master' of https://github.com/MarkBind/markbind

49d8fe2

Merge branch 'master' of https://github.com/MarkBind/markbind

32b5475

Merge branch 'MarkBind:master' into master

ddfa24a

Merge branch 'master' of https://github.com/MarkBind/markbind

2537a1f

Merge branch 'master' of https://github.com/MarkBind/markbind

243df4a

Merge branch 'master' of https://github.com/MarkBind/markbind

9dbdeef

Merge branch 'master' of https://github.com/MarkBind/markbind

7d37a43

Merge branch 'master' of https://github.com/MarkBind/markbind

6423544

Merge branch 'master' of https://github.com/MarkBind/markbind

de29400

remove .mbd and .mbdf from core

1651eb4

remove .mbd and .mbdf helper files

ac46f34

ong6 changed the title ~~Pulled from Master~~ Remove .mbdf and .mbd Feb 12, 2022

ong6 added 5 commits February 13, 2022 00:31

remove .mbd and .mbdf

3f9dd74

Merge branch 'remove-mbd/mbdf' of https://github.com/ong6/markbind in…

8602029

…to remove-mbd/mbdf

remove .mbd and .mbdf

3a857fe

Merge branch 'remove-mbd/mbdf' of https://github.com/ong6/markbind in…

1545bfe

…to remove-mbd/mbdf

removed .mbd and .mbdf from site.json files

74dfc1c

ong6 added 3 commits February 13, 2022 02:00

fixed docs and renamed files

b34f371

renamed files and updated docs

48e74f6

Fixed cli test cases

4d6abca

ong6 marked this pull request as ready for review February 13, 2022 07:25

Merge branch 'master' of https://github.com/MarkBind/markbind into re…

fff09c9

…move-mbd/mbdf

ong6 marked this pull request as draft February 13, 2022 07:33

remove traces of mbd from docs

2d9395a

added sample fragment exclude test

41219d8

ong6 and others added 2 commits February 14, 2022 09:51

Update docs/userGuide/reusingContents.md

8f19ac2

Co-authored-by: Liu YongLiang <41845017+tlylt@users.noreply.github.com>

Update docs/userGuide/reusingContents.md

65b0c37

Co-authored-by: Liu YongLiang <41845017+tlylt@users.noreply.github.com>

ong6 added 2 commits February 14, 2022 09:55

Merge branch 'master' of https://github.com/MarkBind/markbind into re…

a91e2bd

…move-mbd/mbdf

removed .mbd from app factory

88274fb

jonahtanjz reviewed Feb 14, 2022

View reviewed changes

ong6 and others added 8 commits February 15, 2022 21:59

Update packages/cli/index.js

0cb82d2

Co-authored-by: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com>

removed brackets for regex checks

1301b51

removed brakets for all regex

b560f06

Merge branch 'remove-mbd/mbdf' of https://github.com/ong6/markbind in…

8874928

…to remove-mbd/mbdf

update globs

67ff4b1

update markdownfile extensions

3ab170f

Revert "removed brackets for regex checks"

5574571

This reverts commit 1301b51.

Fixed minor bug with test

8649004

jonahtanjz reviewed Feb 16, 2022

View reviewed changes

ong6 and others added 5 commits February 16, 2022 12:09

Update packages/cli/test/functional/test_site/sub_site/nested_sub_sit…

7f9306d

…e/testNunjucksPathResolvingInclude.md Co-authored-by: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com>

Merge branch 'master' of https://github.com/MarkBind/markbind into re…

1c2cc43

…move-mbd/mbdf

Merge branch 'remove-mbd/mbdf' of https://github.com/ong6/markbind in…

9523f4e

…to remove-mbd/mbdf

Fixed test case

2a8c7ce

update isMarkdownFiles

371e5ed

jonahtanjz reviewed Feb 16, 2022

View reviewed changes

packages/cli/index.js Outdated Show resolved Hide resolved

ong6 and others added 2 commits February 16, 2022 22:41

Update packages/cli/index.js

97a3549

Co-authored-by: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com>

Improved resuingContent docs

ece9eec

jonahtanjz approved these changes Feb 16, 2022

View reviewed changes

jonahtanjz added this to the 4.0 milestone Feb 16, 2022

jonahtanjz merged commit 0554eda into MarkBind:master Feb 16, 2022

jovyntls mentioned this pull request Mar 6, 2022

Remove .mbd and .mbdf nus-cs3281/2022#32

Open

jonahtanjz added the breakingChange 💥 Feature will behave significantly different, or is made obsolete label Apr 30, 2022

Remove .mbdf and .mbd #1762

Remove .mbdf and .mbd #1762

Uh oh!

Conversation

ong6 commented Feb 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

damithc commented Feb 12, 2022

Uh oh!

ong6 commented Feb 12, 2022

Uh oh!

damithc commented Feb 13, 2022

Uh oh!

jonahtanjz commented Feb 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ong6 commented Feb 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahtanjz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ong6 commented Feb 15, 2022

Uh oh!

jonahtanjz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonahtanjz Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

ong6 Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

jonahtanjz Feb 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ong6 Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

jonahtanjz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ong6 commented Feb 16, 2022

Uh oh!

jonahtanjz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ong6 commented Feb 12, 2022 •

edited

Loading

jonahtanjz commented Feb 13, 2022 •

edited

Loading

ong6 commented Feb 14, 2022 •

edited

Loading

jonahtanjz Feb 16, 2022 •

edited

Loading