Thread (42 messages) 42 messages, 7 authors, 2021-06-17

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help