Am 07.10.23 um 10:20 schrieb Junio C Hamano: > * rs/parse-options-value-int (2023-09-18) 2 commits > - parse-options: use and require int pointer for OPT_CMDMODE > - parse-options: add int value pointer to struct option > > A bit of type safety for the "value" pointer used in the > parse-options API. > > What's the status of this thing? > source: <e6d8a291-03de-cfd3-3813-747fc2cad145@xxxxxx> tl;dr: Feel free to drop this topic if it's in the way. I think the added _int variables are defensible, but the mode_int member added to struct resume_mode in builtin/am.c is too ugly. Extending OPT_CMDMODE to support arguments would allow getting rid of the struct altogether and is probably a good idea anyway. It's surprisingly hard to get right, though -- just discovered that the interference detection is only working half of the time in the current code, and we can't have that, can we? # a OPT_CMDMODE option $ t/helper/test-tool parse-options --mode1 | grep integer integer: 1 # something else setting the same variable $ % t/helper/test-tool parse-options --set23 | grep integer integer: 23 # combined use is flagged if the OPT_CMDMODE option comes last $ t/helper/test-tool parse-options --set23 --mode1 error: option `mode1' : incompatible with something else # ... but not the other way around $ t/helper/test-tool parse-options --mode1 --set23 | grep integer integer: 23 Anyway, I'll get there eventually and present a nicely shaven yak -- or give up and focus on the original topic. https://lore.kernel.org/git/6cb09270-04b9-456e-8d7e-97137e56e9e2@xxxxxx/ detects and handles the same type mismatch by adding a level of abstraction (getters and setters). That way is more general and adds more overhead compared to the _int variant. A fair comparison requires argument support in OPT_CMDMODE, though, I think. I don't mind removing this topic from seen for now; it's not ready, yet. René
