From: Christian Couder Date: Tue, 11 Feb 2014 15:02:40 GMT Subject: Re: [PATCH v5 02/14] trailer: process trailers from file and arguments Message-ID: In-Reply-To: On Mon, Feb 10, 2014 at 9:51 PM, Junio C Hamano wrote: > Christian Couder writes: > >> Many entries with the same key but distinct values can be configured >> using: >> >> if_exists = add_if_different >> if_missing = add >> >> Many entries with the same key but values that can be the same can be >> configured using: >> >> if_exists = add >> if_missing = add > > While the above certainly can express the combinations, I am still > puzzled about the value of having "under this condition" (i.e. > if-exists/if-missing) and "do this" (i.e. add/add-if-different) as > two separate concepts. It is because there are many possible conditions under which the user might want to do different things, and the syntax of our configuration files is not a programming language, so it is not well suited to nicely express what the user might want under different conditions. That's what I tried to show in my last message in the thread I sent the link to in my previous message. (http://thread.gmane.org/gmane.comp.version-control.git/237278/) So we have to choose a few conditions to avoid possible combinatorial explosion in the number of possible values. > If you do not have an existing entry with the same key, no new entry > can be the same as an existing one---therefore, the former "allow > only distinct values for the same key" can just say > > trailer."Fixes".action = add_if_different > > or something, without any if_exists/if_missing. In a similar way, > the latter "duplicated values allowed for the same key" can say > > trailer."Sob".action = add > > You can throw into the mix other actions like "add_if_missing", you > can also express "only one value allowed for the same key---the > first one wins", "replace" to mean "replace if there is one with the > same key", and "replace_or_add" to mean "same as 'replace', but add > if there is no existing entries with the same key". Will we lose > expressiveness by simplifying the semantics, getting rid of this > "under this condition" part and instead making "do this" part more > intelligent? I don't think we will lose expressiveness, but I think there might be a combinatorial explosion in the number of choices. Right now there are 2 choices for "if_missing" and 5 choices for "if_exists". This means that if we use only one "action" config variable and want to have the same expressiveness, we need 10 choices. It doesn't seem a big difference now, but, if we add more choices, then the difference will increase a lot. > Even if we assume, for the sake of discussion, that it *is* a good > idea to separate "under this condition" part and "do this" part, I > do not think the above is the only or the best way to express > "distinct values allowed for the same key". How do we argue that > this from your example > > if_exists = add_if_different > if_missing = add > > is a better design than this, for example? > > if_key_value_exists = ignore ; exact matching exists > if_key_exists = add ; otherwise > if_missing = add The question is what happens if we want to say "if the same pair exists but not near where we would add. With the solution I implemented, that is: if_exists = add_if_different_neighbor With the solution you suggest, should we have: if_key_value_exists_not_neighbor = add ; exact matching exists but not near where we would add if_key_value_exists = ignore ; exact matching exists if_key_exists = add ; otherwise if_missing = add or: if_key_value_exists = ignore_if_neighbor ; exact matching exists if_key_exists = add ; otherwise if_missing = add And what happens if we want to say "if key exists and value matches ", how do we express that then? Or what happens when we want to say "if key exists and succeeds"? With what I suggest, we can still say: if_exists = add_if_values_matches or if_exists = add_if_succeeds With the solution you suggest, should it be: if_key_value_exists = ignore if_key_exists = add_if_values_matches and if_key_value_exists = ignore if_key_exists = add_if_succeeds ? Doesn't it look like redondant between the 2 lines? And then people might ask if they can also use something like this: if_key_value_exists = add_if_succeeds if_key_exists = add_if_succeeds if_key_missing = add_if_succeeds and it will look like we are trying too much to do what should be done in only one script. > The latter splits remaining conditional logic from "do this" part > (no more "add_if_different" that causes "add" action to see if there > is the same key and act differently) and moves it to "under this > condition" part. > > I am trying to illustrate that the line to split the "under this > condition" and "do this" looks arbitrary and fuzzy here, and because > of this arbitrary-ness and fuzziness, it is not very obvious to me > why it is a good idea to have "under this condition" as a separate > concept from "do this" settings. > > What is the advantage of having such an extra axis? Does it make it > easier to manage? Does it allow us to express more? It will be complex anyway, but at least, with what I implemented, there are only 2 cases in the "under this condition part" and they are completely disjoint, which means that they are hopefully not very difficult to understand. I agree that it may look arbitrary and fuzzy to split things this way, but I think it's a good compromise between avoiding combinatorial explosion in the number of choices, keeping it easy to understand and allowing many features now and more in the future. Thanks, Christian.