Re: [PATCH 5/7] xdiff: rename XDL_MERGE_STYLE_DIFF3
flat view
From: Felipe Contreras <hidden>
Date: 2021-06-11 13:43:56
Junio C Hamano wrote:
Phillip Wood [off-list ref] writes:quoted
The subject would make more sense as 'xdiff: rename XDL_MERGE_DIFF3 to XDL_MERGE_STYLE_DIFF3' rather than using the new name of the constant alone.True.
But why? When we look back in history few people would care what the previous name of XDL_MERGE_STYLE_DIFF3 was, and if they do, they don't necessarily need it in the title.
quoted
quoted
If we don't specify we are talking about a style, XDL_MERGE_MINIMAL could be confused with a valid value instead of XDL_MERGE_DIFF3, which it isn't.I don't object to the rename but what is the source of the confusion with XDL_MERGE_MINIMAL?I do not see any confusion, either, but the current XDL_MERGE_DIFF3 being a boolean
But it's not a boolean: git_xmerge_style is currently -1 by default.
(i.e. if false, use the output style of the 'merge' command) and our lack of an enumeration constant for 'merge' means that a future addition of the third output style would require us to add XDL_MERGE_$STYLE for both the new style and the traditional 'merge' style. And If we would end up with XDL_MERGE_DIFF3, XDL_MERGE_MERGE and XDL_MERGE_FOO for that third output style.
But can you put XDL_MERGE_FOO in xmp.level? Or XDL_MERGE_BAR in xmp.style?
The 'merge' one simply looks strange in that context. And from that point of view, this change might be a good way to futureproof the codebase.
Yes. -- Felipe Contreras