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.

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

Simplify Options and Attributes creation #1015

Conversation

abelsromero
Copy link
Member

@abelsromero abelsromero commented Apr 18, 2021

Make compatible changes to guide users for future changes

Thank you for opening a pull request and contributing to AsciidoctorJ!

Please take a bit of time giving some details about your pull request:

Kind of change

  • Bug fix
  • New non-breaking feature
  • New breaking feature
  • Documentation update
  • Build improvement

Description

What is the goal of this pull request?

Make the experience interacting with Asciidoctor API simpler and less confusing, while mantaining compatibility with current methods and types.

How does it achieve that?
Marks non-prefered methods as deprecated, so users can strt applying changes.

Are there any alternative ways to implement this?
In terms of making the changes smooth no.

Are there any implications of this pull request? Anything a user must know?
No

Issue

If this PR fixes an open issue, please add a line of the form:

Related to #977 but does not close it completely.

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

@abelsromero
Copy link
Member Author

I must say, IMHO, that with this changes the interaction is much nicer to the point that I think most new users should be able to do a test without checking the docs.

@abelsromero
Copy link
Member Author

This reduces the number of convert methods from 21 to 7 😄

@abelsromero
Copy link
Member Author

I saw for load we only accept Maps

/**
* Loads AsciiDoc content and returns the Document object.
*
* @param content to be parsed.
* @param options a Map of options to control processing (default: {}).
* @return Document of given content.
*/
Document load(String content, Map<String, Object> options);
/**
* Loads AsciiDoc content from file and returns the Document object.
*
* @param file to be parsed.
* @param options a Map of options to control processing (default: {}).
* @return Document of given content.
*/
Document loadFile(File file, Map<String, Object> options);
. I preffer to deal with that in a separate PR after this since it will require some real code and more tests.

@robertpanzer robertpanzer merged commit 93f2b6f into asciidoctor:main Apr 20, 2021
@robertpanzer
Copy link
Member

Thank you for this, this is great!
I'd like to make a new release soon including this.
Imo these changes justify a new minor release, i.e. 2.5.0.
What do you think?

@robertpanzer
Copy link
Member

I preffer to deal with that in a separate PR after this since it will require some real code and more tests.

...and add a PR that makes Asciidoctor.load() accept an instance of Options.

@abelsromero
Copy link
Member Author

...and add a PR that makes Asciidoctor.load() accept an instance of Options.

Exactly 😄 just let me add that later today and we should be good to go!

@abelsromero abelsromero deleted the issue-142-backward-compatible-changes-to-simplify-Options-and-Attributes branch December 23, 2021 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants