Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes: > These options have on thing in common: when specified right after
one thing. > --exclude, they will de-select refs instead of selecting them by > default. > > This change makes it possible to introduce new options that use these > options in the same way as --exclude. Such an option would just > implement something like handle_refs_pseudo_opt(). > > parse_ref_selector_option() is taken out of handle_refs_pseudo_opt() so > that similar functions like handle_refs_pseudo_opt() are forced to > handle all ref selector options, not skipping some by mistake, which may > revert the option back to default behavior (rev selection). I am not sure about these two refactorings, for at least two reasons. * Naming. The function is all about handling "refs options" and I do not see anything "pseudo" about the options handled by the handle_refs_pseudo_opt() function. This is minor. * I am expecting that the new one yet to be introduced will not share the huge "switch (selector)" part, but does its own things in a separate function with a similar structure. The only thing common between these two functions would be the structure (i.e. it has a big "switch(selector)" that does different things depending on REF_SELECT_*) and a call to clear_* function. If we were to add a new kind of REF_SELECT_* (say REF_SELECT_NOTES just for the sake of being concrete), what changes will be needed to the code if the addition of "use reflog from this class of refs for decoration" feature was done with or without this step? I have a suspicion that the change will be simpler without this step. The above comments will be invalid if the new "use reflog for decoration" feature will be done by extending this pseudo_opt() function by extending each of the switch/case arms. If that is the case, please disregard the above. I just didn't get that impression from the above paragraph.
