On Sun, Nov 12, 2023 at 11:05:18AM +0000, Gavin Smith wrote: > > However, I doubt that it is justified to rename it. We know it is an > option, so what is the point of putting _OPTION on the end to get > PACKAGE_VERSION_OPTION?
Thinking more about it, I think that there is a point in having a different name. The name PACKAGE_VERSION already refers to something, to the value set by configure. The addition of _OPTION makes clear that it may be modified/customized compared to what configure passed. And it already is modified if TEST is set. Instead of changing that, I propose to also provide a PACKAGE_VERSION in perl that is not an option and is unmodified and can always be used to get what was set by configure. It could make sense to put the information somewhere else than in the customization, and use something similar to what is in the API with get_info(), which is not modifiable (in contrast with get_conf()/set_conf()). This could be available in init files with texinfo_get_info(). In C/XS the configure set macros could be used directly, or the values set in perl could be passed too, and available through get_info(), although the issue with the macro already defined would pop up again if it is a filed in a structure. This could be somewhat confusing, but would allow to distinguish the values that should be used to do portability codes and the values used to output strings. > This could be dealt with in a variety of ways. The most straightforward > would be to "#undef PACKAGE_VERSION" at the beginning of option_types.h. > That way, any source code file using the OPTIONS type and including > this file will not have that preprocessor definition active throughout > the file. If we do that, we should keep the existing value in another macro, renamed, for example PACKAGE_VERSION_CONFIG or PACKAGE_VERSION_MACRO. But the more I think about the more it seems relevant to me to keep PACKAGE_VERSION as set by configure and use a different name for the customization option as done now. > We do not use the PACKAGE_VERSION from the build system for the XS code > and it simply has the value "0". (In the top-level build system, it is > the version of the package, currently "7.1dev".) What we should have here is not fully clear to me, but I think that it should be the value set by configure. Probably having the same as in the main configure would be the best, but we could also have a number that is only incremented if there were changes, like we do for the texinfo DTD. > It's also not necessary that we use the symbol PACKAGE_VERSION in the XS/C > code to refer to this option, so could be renamed to o_PACKAGE_VERSION > as a special case if the #undef option does not work or is not appropriate > for some reason. To me PACKAGE_VERSION_OPTION is better than o_PACKAGE_VERSION, it is clearer, it minimizes the differences between perl variables name and C/XS names used, and between C/XS and documentation. -- Pat
