Recommend zdiff3 merge.conflictStyle

[adamchainz]

Compared to diff3, zdiff3 minimizes the conflicted region, so fewer lines will show as conflicted. This normally makes merge conflicts easier to resolve. Docs: https://git-scm.com/docs/git-config#Documentation/git-config.txt-mergeconflictStyle .

I also made conflictStyle follow Git’s casing, as other options are documented like that as well.

[th1000s]

The option zdiff3 is rather new, added in 2.35 from last year. So this change would break systems with older git versions.

[adamchainz]

True - Git 2.35 was released 2022-01-24. But I'm not sure a >1 year old version is such a concern... Users on such old versions also have many unpatched CVEs, since Git doesn't backport security fixes too far 🤷‍♂️

[simono]

@dandavison Would you consider to merge this change? Thanks 😄

[dandavison]

100% happy to add the recommendation to the manual; no hesitation there (does anyone want to come up with good "before and after" examples/screenshots showing the advantage?).

As for the front-page recommended config, we don't have an explicit policy yet on compatibility with Git versions. I think this could be viewed as a statistical question: where does the Git version that this requires fall in the distribution of Git versions used by delta users? Which leads to the question: what version of Git do the most common operating systems ship with or in the cases where people get git from package managers what do they get? (I don't think CVEs in Git are relevant; people use whatever version of Git they have.)

[simono]

Maybe we can link to this GitHub blog post, which explains zdiff3 pretty well?

zdiff3 was introduced in Git 2.35 (Jan 2022). I think it should be shipped with all major operating systems and package managers by now.

Some examples:
macOS has git 2.39 since at least Version 13 (Ventura).
Debian has git 2.39 since Version 12 (Bookworm), with backports available for older systems.
Ubuntu has git 2.40 since Version 23.10 (Mantic Minotaur).

[dandavison]

Thanks @simono, I read the blog post example, but I admit I was waiting until I could personally started noticing the improvements in my own workflows.

This may be old news to everyone else but for the record, here's an example. It's simpler than the one in the GitHub blog -- in a way there is no conflict, just two adjacent changes. So this one's a big improvement because under zdiff3 in my Git UI (magit/vscode/lazygit etc) I can just stage it without resolving anything, whereas diff3 would require me to choose a resolution.

with diff3:

with zdiff3:

diff --cc service/frontend/workflow_handler.go
index a3df8fc7e,8484f2a3e..000000000
--- a/service/frontend/workflow_handler.go
+++ b/service/frontend/workflow_handler.go
@@@ -2157,34 -2149,34 +2165,42 @@@ func (wh *WorkflowHandler) ResetWorkflo
  }
  
  // TerminateWorkflowExecution terminates an existing workflow execution by recording WorkflowExecutionTerminated event
  // in the history and immediately terminating the execution instance.
  func (wh *WorkflowHandler) TerminateWorkflowExecution(ctx context.Context, request *workflowservice.TerminateWorkflowExecutionRequest) (_ *workflowservice.TerminateWorkflowExecutionResponse, retError error) {
  	defer log.CapturePanic(wh.logger, &retError)
  
  	if request == nil {
  		return nil, errRequestNotSet
  	}
  
  	if err := validateExecution(request.WorkflowExecution); err != nil {
  		return nil, err
  	}
  
++<<<<<<< HEAD
 +	if err := wh.validateLinks(namespace.Name(request.GetNamespace()), request.GetLinks()); err != nil {
 +		return nil, err
 +	}
 +
++||||||| parent of 9f57b1a51 (Instrument some more endpoints)
++=======
+ 	trace.SpanFromContext(ctx).SetAttributes(
+ 		telemetry.WorkflowIDKey(request.WorkflowExecution.WorkflowId),
+ 	)
+ 
++>>>>>>> 9f57b1a51 (Instrument some more endpoints)
  	namespaceID, err := wh.namespaceRegistry.GetNamespaceID(namespace.Name(request.GetNamespace()))
  	if err != nil {
  		return nil, err
  	}
  
  	_, err = wh.historyClient.TerminateWorkflowExecution(ctx, &historyservice.TerminateWorkflowExecutionRequest{
  		NamespaceId:      namespaceID.String(),
  		TerminateRequest: request,
  	})
  	if err != nil {
  		return nil, err
  	}
  
  	return &workflowservice.TerminateWorkflowExecutionResponse{}, nil
  }

diff --cc service/frontend/workflow_handler.go
index a3df8fc7e,8484f2a3e..000000000
--- a/service/frontend/workflow_handler.go
+++ b/service/frontend/workflow_handler.go
@@@ -2157,33 -2149,33 +2165,36 @@@ func (wh *WorkflowHandler) ResetWorkflo
  }
  
  // TerminateWorkflowExecution terminates an existing workflow execution by recording WorkflowExecutionTerminated event
  // in the history and immediately terminating the execution instance.
  func (wh *WorkflowHandler) TerminateWorkflowExecution(ctx context.Context, request *workflowservice.TerminateWorkflowExecutionRequest) (_ *workflowservice.TerminateWorkflowExecutionResponse, retError error) {
  	defer log.CapturePanic(wh.logger, &retError)
  
  	if request == nil {
  		return nil, errRequestNotSet
  	}
  
  	if err := validateExecution(request.WorkflowExecution); err != nil {
  		return nil, err
  	}
  
 +	if err := wh.validateLinks(namespace.Name(request.GetNamespace()), request.GetLinks()); err != nil {
 +		return nil, err
 +	}
+ 	trace.SpanFromContext(ctx).SetAttributes(
+ 		telemetry.WorkflowIDKey(request.WorkflowExecution.WorkflowId),
+ 	)
  
  	namespaceID, err := wh.namespaceRegistry.GetNamespaceID(namespace.Name(request.GetNamespace()))
  	if err != nil {
  		return nil, err
  	}
  
  	_, err = wh.historyClient.TerminateWorkflowExecution(ctx, &historyservice.TerminateWorkflowExecutionRequest{
  		NamespaceId:      namespaceID.String(),
  		TerminateRequest: request,
  	})
  	if err != nil {
  		return nil, err
  	}
  
  	return &workflowservice.TerminateWorkflowExecutionResponse{}, nil

[ccoVeille]

zdiff3 was a game changer for me

[dandavison]

@th1000s do you think enough time has passed to put this in the example config that people will blindly copy and paste, or should we just add it as a recommendation for now?

[th1000s]

Okay, on an incompatible setting it causes the next git pull/rebase command to fail, even if there was no conflict: fatal: unknown style 'zdiff3' given for 'merge.conflictstyle'. For LTS or other users who use older versions the cause should be obvious.

[dandavison]

OK, thanks all! Merging this after a 2-year docs PR review process :)

[adamchainz]

Yay, thanks!