Jeff King <peff@xxxxxxxx> writes: >> You can certainly argue that this "textconv" feature that is grafted from >> Porcelain into plumbing is a special case in that its output is subject to >> change any time to help human consumption and we never strive for its >> stability as we do for other features in the plumbing to support machine >> readability by scripts. You can propagate the later enhancement of >> textconv diff output we'd make for Porcelain to the scripted users that >> reads from the plumbing that way. > > Right. _If_ it's a change that won't upset any plumbing consumers. If it > is, then it needs to be a separate option so that the plumbing consumers > who don't mind the change can start using it. In the above argument, you are assuming that you can have a complete enumeration of "plumbing consumers", so that we can tell what kind of changes to textconv output do affect them in a negative way, and what kind of changes are safe. Yes, you can enumerate in-tree consumers, but that misses the whole point of having "plumbing", which can be used by any scripts out of tree and they can rely on stable output from the plumbing. By giving --textconv to them to use with the plumbing, you are effecively casting textconv output in stone. By definition we will never know what kind of changes to the output from the textconv filter out of tree consumers would mind. Currently "diff" machinery uses the output of textconv filter as-is without any further embellishment, but I think it is too early to tell if that is really what we would want, or we may need some minimum postprocessing on what we receive from the textconv filter before feeding that to the textual diff machinery, because the feature has not been used in the field at all yet. In any case, I've applied the series for an entirely different reason. The patch is the most natural way to allow users of Porcelain to disable textconv with the --no-textconv option, just like --no-ext-diff can be used to disable the external diff. We _might_ want to add another patch to make the plumbing layer ignore (or error out) --textconv option given from the command line before 1.6.1 ships, and after we gain confidence with the stability of the feature, revert that patch to allow use of --textconv freely from the plumbing. It always is easier to introduce a new feature in a more limited form and then later loosen it, than adding it in an unrestricted form and having later to limit it for whatever reason. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
