Expose Section's sectnum property

[abelsromero]

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?
Exposes sectnum to users using the API for extensions, converters, etc.

This property is required to easily obtain the section numbering aligned with other Asciidoctor attributes. The alternative is reimplementing all the logic of level counting, secnumlevel, separators formating, etc.

How does it achieve that?

Adds 2 new methods:

• String getSectnum()
• String getSectnum(String delimiter)

to the Section interface, those simply call the Asciidoctor Ruby property (I assume property is the right name, it's a method).

Are there any alternative ways to implement this?

While working on a converter I could not find it. index returns 0, and there's a lot of reimplementation to do to obtain a similar result.

Right now I am using internals String sectnum = ((SectionImpl) node).getString("sectnum"), which is not ideal.

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

1. The property in Asciidoctor is well documented as public, I am assuming this is stable and can be used, but could be wrong. @mojavelinux can you confirm? 🙇
2. I am keeping the default behauviour when sectnums is not set, that is, the delimiters without numbers is returned (see test should_return_invalid_sectnum_when_sectionNumbers_are_not_enabled). Is odd and I am tempted to add a check and return empty string, but maybe this should be fixed upstream? //cc @mojavelinux
3. I am also not adding support to pass the append parameter, to keep things simple, and also adding a suffix is something that API users can just do just outside of the method.

Issue

n/a

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

[robertpanzer]

Awesome, thank you!
It looks good to me.
I'll wait for feedback from @mojavelinux

@abelsromero Can you shed some light on the append parameter? I am not sure if I understood what you meant.

[abelsromero]

@abelsromero Can you shed some light on the append parameter? I am not sure if I understood what you meant.

If you look at the documentation in the Ruby implementation, the extra append param says...

the String to append at the end of the section number
             or Boolean to indicate the delimiter should not be
             appended to the final level

so, it allows the behaviours:

• By default (null, not set, or true) it will appent the delimiter so we get something like "1.1." (see final dot.)
• When false, won't append delimiter -> we get "1.1"
• When set to a string, this is appended -> for example calling with "_", we get "1.1_"

I personally think (but it's subjective 😄) these options do not add much in terms of features. Any user that requires this level of fine tunning can do so outside of the method with normal String manipulations. And if in the future this is requested, we can always add it based on a real use case.

[abelsromero]

Checking the errors in test-upstream, I am not sure the reason. Locallly works fine and the CI logs show the version installed being Version: 2.1.0.pre.alpha.0, not sure where the later SNAPSHOT in same ci execution comes from.

inconsistent module metadata found. Descriptor: rubygems:asciidoctor:2.1.0.pre.alpha.0-SNAPSHOT Errors: bad version: expected='2.1.0-alpha.0' found='2.1.0.pre.alpha.0-SNAPSHOT'

[robertpanzer]

I guess this behavior might be related to this:
https://twitter.com/mojavelinux/status/1575764592675811328?s=20&t=St8K5u1adGt9-pJcR9veBQ

[mojavelinux]

Yep, I need to switch core back to using dot to separate the prerelease component in the version because, once again, Bundler changed its rules and our workaround no longer works.

[abelsromero]

Sorry to be insistent, but:

• locally works fine (running the script or running line by line), at least in my Linux machine
• logs show the gem is installed locally and the version

So , if I understand correctly the gem plugin is doing some extra remote check that is not happenign in my machine, and rubygems returns a different version number? If so, maybe we can disable that check.

[mojavelinux]

The fix has been applied upstream, so now it should return the version number as entered in version.rb. See https://github.com/asciidoctor/asciidoctor/blob/main/lib/asciidoctor/version.rb. So if you're still seeing 2.1.0-alpha.0, then its being picked up from the cache (that version isn't actually released).

[mojavelinux]

It appears like there are other problems:

inconsistent module metadata found. Descriptor: rubygems:asciidoctor:2.0.18.pre-SNAPSHOT Errors: bad version: expected='2.0.18.pre' found='2.0.18.pre-SNAPSHOT'

I don't know what the source of this check is, but it is clearly choking on this problem I alluded to in gem versioning. RubyGems doesn't allow the use of a hyphen, but semver doesn't allow a use of the hyphen after the third dot. It might be necessary to put some hacks somewhere in this build to deal with this incompatibility. The quick thing to do would be to change the version in version.rb to 2.0.18 after cloning.

[abelsromero]

I don't know what the source of this check is

gem-maven-plugin is doing it. The '-SNAPSHOT' is added to the pom descriptor's <version> but the artifact is installed under .m2/repository/rubygems/asciidoctor/2.1.0.alpha.0 (without it). That's why gradle mentions finding 2.0.18.pre-SNAPSHOT.

I found 3 solutions

1. Append the -SNAPSHOP tail before installation, here.
2. Append the -SNAPSHOP tail during test, here.
3. Extracting the version from the installed pom and passing it during tests.

I'd say the third option is the secure one, unless we understand why the snapshot is added.

PS: tested bumping gem-maven-plugin, and it complains about some encoding issue in the pom I could not correct. Some misterious characters 🤷

[abelsromero]

Rebased the fix for upstream tests, we should be seeing 🟢 now

[robertpanzer]

Thank you!