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.

Sign up for GitHub

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

Jump to bottom

Support .asciidoctorconfig at root of the workspace #636

Merged
merged 3 commits into from
Oct 4, 2022
Merged

Support .asciidoctorconfig at root of the workspace #636

merged 3 commits into from
Oct 4, 2022

Conversation

apupier
Copy link
Member

@apupier apupier commented Sep 9, 2022 •
edited
Loading

part of #380

demo asciidortorconfigAtRootWorkspaceFolder

TODO for this PR:

  • provide test

TODO for another iteration:

  • support other places
  • support .asciidoctorconfig.adoc
  • provide a cache mechanism to avoid reading config file every time

Might not have time to work on it before 30th September but wanted to share current state

ggrossetie
ggrossetie reviewed Sep 9, 2022
View reviewed changes
src/asciidocParser.ts Outdated Show resolved Hide resolved
ggrossetie
ggrossetie reviewed Sep 11, 2022
View reviewed changes
src/asciidocParser.ts Outdated Show resolved Hide resolved
@apupier apupier force-pushed the 380-asciidoctorconfig branch from 1f62c12 to 166fa1c Compare September 30, 2022 12:05
@apupier
Support .asciidoctorconfig at root of the workspace
a188efb 
part of asciidoctor#380

TODO in this PR:
- provide test

TODO in other iteration:
- support other places
- support `.asciidoctorconfig.adoc`
- provide cache mechanism
@apupier apupier force-pushed the 380-asciidoctorconfig branch from 166fa1c to a188efb Compare September 30, 2022 12:26
@apupier
Copy link
Member Author

apupier commented Sep 30, 2022 •
edited
Loading

Updates:

Provided:

  • .ascidoctorconfig support when at root of the workspace with double \n and
  • changelog
  • basic doc

TODO for this PR (any hints is welcome):

  • provide test
  • handle the preprocessor reader.push_include

TODO for another iteration:

  • support other places
  • support .asciidoctorconfig.adoc
  • provide a cache mechanism to avoid reading config file every time

My plan is to look at it again on 7th October. If someone wants to pick up where I left (or even restart from scratch) to provide the .asciidoctorconfig support, this is perfectly fine.

@ggrossetie ggrossetie marked this pull request as ready for review October 2, 2022 12:29
@ggrossetie
Copy link
Member

That's a good plan @apupier

I fixed the test and introduce a Preprocessor to push the content of the .asciidoctorconfig at the start of the document.
As a side note, we desperately need to simplify the code as we have a lot of boilerplate. I believe it will make things easier.

I think it's ready to go. @ahus1 @apupier If you have time to review the code that would be awesome.

@ggrossetie ggrossetie mentioned this pull request Oct 2, 2022
Open
apupier
apupier commented Oct 3, 2022
View reviewed changes
Copy link
Member Author

@apupier apupier left a comment

Choose a reason for hiding this comment

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

tested locally and it works fine.

Thank you very much for the addition of the test and the preprocessor!

(I cannot approve as I created the Pull request)

src/features/asciidoctorConfig.ts Show resolved Hide resolved
@ggrossetie ggrossetie merged commit e454a31 into asciidoctor:master Oct 4, 2022
@apupier apupier deleted the 380-asciidoctorconfig branch October 4, 2022 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggrossetie ggrossetie ggrossetie left review comments

@ahus1 ahus1 ahus1 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apupier @ggrossetie @ahus1