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

Kill off alphabetical ordering clause for publicly-exposed enum defs #851

Closed
jengelh opened this issue Dec 16, 2019 · 2 comments
Closed

Comments

@jengelh
Copy link

jengelh commented Dec 16, 2019

While working on #850 , I found this problematic statement in README/OPTIONS.md:

In `tidyenum.h` the `TidyOptionId` can be in any order, but please try to keep things alphabetical

When adding an option in the middle, all other option ids get pushed down (effectively incremented by one). This would break binary compatibility and expectations of existing programs, for example when such a program invokes:

tidyGetOption(td, TidyCSSPrefix /* 10 */);

If I were to add a a TidyAaaargh option and retaining alphabetical order, TidyCSSPrefix would become 11, and the program would receive, from tidyGetOption, the option for TidyCoerceEndTags (new 10).

I therefore seek reaffirmation that removing the "do it alphabetical" clause is the right thing to do, lest the SO version of the library must be bumped almost everytime such a new option is added.

The same holds true for any other enum that is publicly exposed, including, but not limited to, TidyStrings.

@geoffmcl
Copy link
Contributor

@jengelh I tend to agree... but fear we have already crossed that line... need to check...

Perhaps it could be reverted to the wording before commit 5f05add, Mar 2017, unless @balthisar has some reason to maintain them alphabetically... aside from maybe Doxygen only...

-In `tidyenum.h` the `TidyOptionId` can be in any order, but normally a new option would be added just before the last `N_TIDY_OPTIONS`, which must remain the last. Choosing the id name can be any string, but by convention it will commence with `Tidy` followed by brief descriptive text.
+In `tidyenum.h` the `TidyOptionId` can be in any order, but please try to keep things alphabetical, and keep in mind that `N_TIDY_OPTIONS` must remain the last. Choosing the id name can be any string, but by convention it will commence with `Tidy` followed by brief descriptive text.

Or perhaps the wording could be stronger, or different...

Look forward to further feedback, especially from @balthisar... thanks...

@balthisar
Copy link
Member

Our only stable releases are even number minor versions, such as 5.6, and the upcoming 5.8. Odd minor versions don't offer ABI/API stability. Unfortunately this means that our not-quite-semantic semantic versioning is off by one place, in that 5.6 and 5.8 should be compatible, but are not. In order to address this, I'm going to update the SONAME to reflect this, essentially by removing the point (5.8 will be SONAME 58).

An alternative would be to move completely to semantic versioning. We were attached to Tidy 5 when we took over from the W3C, because HTML 5 support was fairly novel at the time. Now six years later, it's pervasive, and we could make this change.

With that explanation, I'm going to close the issue, however.

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

No branches or pull requests

3 participants