Hey, Sorry for sending it over and over again. I couldn't figure out why the mail is not reaching the community. > > Oops? > Sorry about this. Would correct it in the next patch. > I'd suggest to follow the excellent first paragraph above with > something like the following. > > Make `--ignore-submodules=untracked` the default for `git > diff` when there is no configuration variable or command > line option, so that the command would not give '-dirty' > suffix to a submodule whose working tree has untracked > files, to make it consistent with `git describe --dirty` > that is run in the submodule working tree. Very well written. Would update the description to this. Thanks. > This new boolean is meant to be set only when non-default > ignore-submodules option is given either from the command line or > from the configuration---when the bit is unset, we are told to do > whatever it is that is the default for us. I fear I might not get your question clearly here. But from what I understood this boolean( flags.ignore_submodule_set ) is set only when diff.ignoreSubmodules is set in user-config. > And the default is to flip only this bit on. Do we need to turn off > other bits that submodule.c::handle_ignore_submodules_arg() function > touches? [*1*] > We are not flipping the bit, we are setting it to 1. I guess we don't have to turn off other bits because in case if submodule.c::handle_ignore_submodules_arg() is called we don't have to set the default value. So there's no point in flipping any other bits as if any of them is set, the user should have added some arguments in ignoreSubmodules and therefore flags.ignore_submodule_set would be set to 1 and we won't be setting the default value. > > I like the general idea of having one bit that is set if and only if > the command line or configuration told us specifically what to do, > so that we can dictate the default after they were taken care of. > > But I am not sure if this is a good implementation of that idea. > > Case in point. I was wondering if the most future-proof way to > answer the question I asked (marked with [*1*] above) was to avoid > flipping the bits in options->flags ourselves, but to make a call > > handle_ignore_submodules_arg(&options, "untracked"); > > in repo_diff_setup(). When such an improvement is made after this > patch lands, the assumption that only the end-user preference will > call this function no longer holds. I tried making the call directly like this: handle_ignore_submodules_arg(&options, "untracked") without using the extra bit, but in the case when the user specifies diff.ignoreSubmodules in user-config, the handle_ignore_submodules_arg() is called even before repo_diff_setup() and therefore the default value overwrites the value mentioned in user-config. > Even without anticipating such a change in the future, there already > is a callsite of this function in wt-status.c that hands a hardcoded > string "dirty" to it. That is *not* caused by an end-user request > (be it a configuration variable or a command line option), so in a > sense, the assumption is already broken. > I couldn't fully understand what assumption are you talking about. I would be glad if you can explain it to me in a little more detail. Thanks! > I wonder, if we can do things in a more natural way (at least the > natural way in this codebase). Usually we do things in this order: > > - initialize the status and option variables to their default state. > > - read the configuration files to allow the state of these > variables to be modified from their default state. > > - parse the command line arguments to further allow the state of > these variables to be modified. > > and then use the final state of these variables. That way, we do > not even need the extra bit that is only required if we did things > in an unnatural way, which is > > - read the config; remember if any bits were toggled > - parse the command line; remember if any bits were toggled > - only if bits weren't toggled in the above, set the default > > I dunno. Ya, that makes sense. I would look into when the config values are being read and bits are set and would try to find a way so that we can get rid of the extra bit. Thanks for pointing this out. > > Noise. > I hope this is a substitute for nice. XD. Hearing this after a long time. I used to hear this more frequently when I was in college. > > Misindented. Noted. Would change that in the next patch. Thanks and regards, Sangeeta
