Skip to content

Resolve git merge conflicts change by change [INS-908] - #9052

Merged
yaoweiprc merged 2 commits into
developfrom
INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file
Sep 5, 2025
Merged

Resolve git merge conflicts change by change [INS-908]#9052
yaoweiprc merged 2 commits into
developfrom
INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file

Conversation

@yaoweiprc

@yaoweiprc yaoweiprc commented Aug 25, 2025

Copy link
Copy Markdown
Contributor
  1. A new resolve conflict modal with a merge editor that empowers users to resolve conflicts, change by change, in Git sync.
image
  1. Add startCollectWriteAction & stopCollectWriteAction functions to the fs object passed to git-vcs to collect conflicted file content when encountering conflicts.

  2. Call abortMerge explicitly after the user cancels resolving conflicts.

  3. Refactor sync-merge-modal.tsx.

@yaoweiprc
yaoweiprc marked this pull request as draft August 25, 2025 09:28
Comment thread packages/insomnia/src/ui/components/modals/sync-merge-modal.tsx Fixed
Comment thread .vscode/settings.json Outdated
Comment thread package.json Outdated
Comment on lines +72 to +73
"@mismerge/core": "^1.2.1",
"@mismerge/react": "^1.0.1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interested how you can justify adding a new package here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don’t want to introduce a new dependency, but in the resolve conflict scenario, this library provides a professional two-way merge editor interaction. Users can view the two conflicting branches side by side and edit the final file content in the middle, with highlighted displays of the conflicting parts. I admit that with heavy customization of the Monaco Editor, we could achieve a VSCode-like experience, but that would require a huge amount of development effort. I believe using this library is the better choice for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library doesn't support react 19

@yaoweiprc
yaoweiprc force-pushed the INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file branch 3 times, most recently from eee41ae to e8f1dff Compare August 27, 2025 10:09
@yaoweiprc
yaoweiprc marked this pull request as ready for review August 27, 2025 17:20
@yaoweiprc
yaoweiprc force-pushed the INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file branch from 1e49505 to bc80b0d Compare August 27, 2025 17:43
@yaoweiprc
yaoweiprc marked this pull request as draft August 28, 2025 02:21
@yaoweiprc
yaoweiprc force-pushed the INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file branch from bc80b0d to 3d30996 Compare August 28, 2025 07:38
@yaoweiprc
yaoweiprc force-pushed the INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file branch 4 times, most recently from 3235e1e to 624b5d0 Compare September 4, 2025 11:25
@yaoweiprc
yaoweiprc marked this pull request as ready for review September 4, 2025 11:54
@yaoweiprc
yaoweiprc force-pushed the INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file branch from 624b5d0 to 7c25f23 Compare September 5, 2025 05:17
mergeConflictError: InstanceType<typeof git.Errors.MergeConflictError>,
oursBranch: string,
theirsBranch: string,
writeFileMap?: WriteFileMap,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I see every call passes this, maybe we could remove the ? to make it simpler.

let isLightTheme = false;
if (rootLoaderData?.settings) {
const colorScheme = getColorScheme(rootLoaderData.settings);
if (colorScheme === 'light') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we consider adding the mismerge theme config to the theme files?

@yaoweiprc
yaoweiprc force-pushed the INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file branch from 7c25f23 to c6c1914 Compare September 5, 2025 07:19
@yaoweiprc
yaoweiprc merged commit 76e8af7 into develop Sep 5, 2025
14 checks passed
@yaoweiprc
yaoweiprc deleted the INS-908-Allow-resolving-conflicts-change-by-change-instead-of-file-by-file branch September 5, 2025 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants