On 1/2/2021 1:12 AM, Eric Sunshine wrote: > On Fri, Jan 1, 2021 at 8:09 AM Derrick Stolee via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: >> My strategy for update-index was to create static globals "repo" and >> "istate" that point to the_repository and the_index, respectively. Then, I >> was able to remove macros one-by-one without changing method prototypes >> within the file. >> >> I had started trying to keep everything local to the method signatures, but >> I hit a snag when reaching the command-line parsing callbacks, which I could >> not modify their call signature. [...] > > You should be able to do this, not by modifying the callback > signature, but by taking advantage of the `extra` member of `struct > option` which is available to callback functions or arbitrary use. If > you need to access the index in a callback, then assign a `struct > index_state *` to `extra`; likewise assign a `struct repository *` to > `extra` to access the repository. If you need access to both the index > and the repository, then just store the repository in `extra` since > the repository has an `index` field. > > You won't be able to use any of the canned OPT_FOO() macros to > initialize an entry in the update-index.c `options[]` array which > needs `extra`-initialization since the macros don't let you specify > `extra`, but you can easily bypass the macro and initialize the > `struct option` manually. (After all, the macros exist for > convenience; they are not a hard requirement.) > > Within the callback, extract the `repository` or `index_state` as you > would any other field. For instance: > > const struct repository *repo = opt->extra; Yes, this is definitely the way to make it possible. > This should allow you to get rid of the globals introduced by patch > [4/12] (assuming passing the index and repo arguments around > everywhere doesn't get overly hairy). My attempts just getting to the point of hitting these callbacks was already making me frustrated with how complicated the code became with that approach. Perhaps now that I've removed the compatibility macros, it would be easier to insert the method parameters since most of the lines that need to change would be method prototypes and the calls to those methods (plus the callback function details). Is that a valuable effort? I could give it a try, but I want to be sure that adjusting all of those helper methods in the builtin would indeed have valuable improvements over the static globals used here. Thanks, -Stolee
