Skip to content

Add option to preserve header#58

Merged
bwplotka merged 3 commits intobwplotka:mainfrom
saswatamcode:preserve-header
Jul 12, 2021
Merged

Add option to preserve header#58
bwplotka merged 3 commits intobwplotka:mainfrom
saswatamcode:preserve-header

Conversation

@saswatamcode
Copy link
Copy Markdown
Collaborator

@saswatamcode saswatamcode commented Jul 5, 2021

This PR changes transform tranformation config to preserve markdown headers by default while transforming as discussed in 1:1. popHeader option may be added to enable removing header.

Note: Could not find a better explicit way of enabling remove header other than bool option in config (as was mentioned in 1:1). Would a flag instead be preferred?

@saswatamcode
Add popHeader option
0a1af8e 
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
@saswatamcode saswatamcode requested a review from bwplotka July 5, 2021 05:18
@saswatamcode saswatamcode self-assigned this Jul 5, 2021
bwplotka
bwplotka approved these changes Jul 5, 2021
View reviewed changes
Copy link
Copy Markdown
Owner

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice LGTM, just one idea: Wonder if we can global default popHeader on root file? 🤔

@saswatamcode
Copy link
Copy Markdown
Collaborator Author

saswatamcode commented Jul 5, 2021

Sure! Will try to implement this!

@saswatamcode
Copy link
Copy Markdown
Collaborator Author

saswatamcode commented Jul 8, 2021

Now, popHeader will be true by default for root files in input dir. This can be disabled by setting popHeader to false with glob matching root file. Also, all root files will have this as true by default, so would need to be configured separately if this behavior is not needed. Test case added in pkg/transform/testdata/mdox3.yaml where popHeader is not set for README.md but md header is still popped.

@saswatamcode
Make root file popHeader default
e6836da 
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
@saswatamcode saswatamcode requested a review from bwplotka July 8, 2021 05:50
bwplotka
bwplotka reviewed Jul 10, 2021
View reviewed changes
pkg/transform/testdata/mdox1.yaml Outdated
transformations:
- glob: "README.md"
path: _index.md
popHeader: true
Copy link
Copy Markdown
Owner

@bwplotka bwplotka Jul 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If popHeader is true on root, why we do this? 🤔

Copy link
Copy Markdown
Collaborator Author

@saswatamcode saswatamcode Jul 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, forgot to remove!

Copy link
Copy Markdown
Owner

@bwplotka bwplotka Jul 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still there on all examples 🤔

@saswatamcode
Fix test
7ba3d3f 
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
@saswatamcode saswatamcode requested a review from bwplotka July 10, 2021 11:38
@bwplotka bwplotka merged commit f49414c into bwplotka:main Jul 12, 2021
@bwplotka
Copy link
Copy Markdown
Owner

bwplotka commented Jul 12, 2021

Merging, LGTM thanks, but examples are still not perfect 🙈

@saswatamcode
Copy link
Copy Markdown
Collaborator Author

saswatamcode commented Jul 12, 2021

I've missed some. Will raise PR to fix them! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bwplotka bwplotka Awaiting requested review from bwplotka

Assignees

@saswatamcode saswatamcode

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@saswatamcode @bwplotka