Skip to content

Commit

Permalink
Support for resolving conflicts when using git-merge [INS-4638] (#8185)
Browse files Browse the repository at this point in the history
* Remove unused codes

* let user resolve conflict when using git merge

* fix code review issue

* Solve the bug that working area does not update after merge.

* modify git smoke test timeout
  • Loading branch information
yaoweiprc authored Nov 25, 2024
1 parent 8703b88 commit bc8aa38
Show file tree
Hide file tree
Showing 9 changed files with 291 additions and 160 deletions.
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@
"inso",
"libcurl",
"mockbin",
"Revalidator",
"svgr",
"Uncommited",
"Unpushed",
"unstage",
"Unstaged",
"upsert",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { test } from '../../playwright/test';

test('Git Interactions (clone, checkout branch, pull, push, stage changes, ...)', async ({ page }) => {
const gitSyncSmokeTestToken = process.env.GIT_SYNC_SMOKE_TEST_TOKEN;
test.setTimeout(60000);
test.setTimeout(600000);

// read env variable to skip test
if (!gitSyncSmokeTestToken) {
Expand Down
60 changes: 47 additions & 13 deletions packages/insomnia/src/sync/git/git-vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -636,9 +636,13 @@ export class GitVCS {
}
}

async pull(gitCredentials?: GitCredentials | null) {
async _hasUncommittedChanges() {
const changes = await this.status();
const hasUncommittedChanges = changes.staged.length > 0 || changes.unstaged.length > 0;
return changes.staged.length > 0 || changes.unstaged.length > 0;
}

async pull(gitCredentials?: GitCredentials | null) {
const hasUncommittedChanges = await this._hasUncommittedChanges();
if (hasUncommittedChanges) {
throw new Error('Cannot pull with uncommitted changes, please commit local changes first.');
}
Expand All @@ -658,7 +662,6 @@ export class GitVCS {
err,
oursBranch,
theirsBranch,
gitCredentials
);
} else {
throw err;
Expand All @@ -667,11 +670,11 @@ export class GitVCS {
);
}

// Collect merge conflict details from isomorphic-git git.Errors.MergeConflictError and throw a MergeConflictError which will be used to display the conflicts in the SyncMergeModal
async _collectMergeConflicts(
mergeConflictError: InstanceType<typeof git.Errors.MergeConflictError>,
oursBranch: string,
theirsBranch: string,
gitCredentials?: GitCredentials | null,
) {
const {
filepaths, bothModified, deleteByUs, deleteByTheirs,
Expand All @@ -691,13 +694,11 @@ export class GitVCS {

const oursHeadCommitOid = await git.resolveRef({
...this._baseOpts,
...gitCallbacks(gitCredentials),
ref: oursBranch,
});

const theirsHeadCommitOid = await git.resolveRef({
...this._baseOpts,
...gitCallbacks(gitCredentials),
ref: theirsBranch,
});

Expand All @@ -706,7 +707,6 @@ export class GitVCS {
function readBlob(filepath: string, oid: string) {
return git.readBlob({
..._baseOpts,
...gitCallbacks(gitCredentials),
oid,
filepath,
}).then(
Expand Down Expand Up @@ -798,6 +798,11 @@ export class GitVCS {
commitParent: string[];
}) {
console.log('[git] continue to merge after resolving merge conflicts', await this.getCurrentBranch());

// Because wo don't need to do anything with the conflicts that user has chosen to keep 'ours'
// Here we just filter in conflicts that user has chosen to keep 'theirs'
handledMergeConflicts = handledMergeConflicts.filter(conflict => conflict.choose !== conflict.mineBlob);

for (const conflict of handledMergeConflicts) {
assertIsPromiseFsClient(this._baseOpts.fs);
if (conflict.theirsBlobContent) {
Expand All @@ -813,21 +818,50 @@ export class GitVCS {
await git.remove({ ...this._baseOpts, filepath: conflict.key });
}
}

// Add other non-conflicted files to the stage area
await git.add({ ...this._baseOpts, filepath: '.' });

await git.commit({
...this._baseOpts,
message: commitMessage,
parent: commitParent,
});
}

async merge(theirBranch: string) {
const ours = await this.getCurrentBranch();
console.log(`[git] Merge ${ours} <-- ${theirBranch}`);
async merge({
theirsBranch,
allowUncommittedChangesBeforeMerge = false,
}: {
theirsBranch: string;
allowUncommittedChangesBeforeMerge?: boolean;
}) {
if (!allowUncommittedChangesBeforeMerge) {
const hasUncommittedChanges = await this._hasUncommittedChanges();
if (hasUncommittedChanges) {
throw new Error('There are uncommitted changes on current branch. Please commit them before merging.');
}
}
const oursBranch = await this.getCurrentBranch();
console.log(`[git] Merge ${oursBranch} <-- ${theirsBranch}`);
return git.merge({
...this._baseOpts,
ours,
theirs: theirBranch,
});
ours: oursBranch,
theirs: theirsBranch,
abortOnConflict: false,
}).catch(
async err => {
if (err instanceof git.Errors.MergeConflictError) {
return await this._collectMergeConflicts(
err,
oursBranch,
theirsBranch,
);
} else {
throw err;
}
},
);
}

async fetch({
Expand Down
9 changes: 8 additions & 1 deletion packages/insomnia/src/sync/git/ne-db-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,14 @@ export class NeDBClient {
return;
}

const doc: BaseModel = YAML.parse(data.toString());
const dataStr = data.toString();

// Skip the file if there is a conflict marker
if (dataStr.split('\n').includes('=======')) {
return;
}

const doc: BaseModel = YAML.parse(dataStr);

if (id !== doc._id) {
throw new Error(`Doc _id does not match file path [${doc._id} != ${id || 'null'}]`);
Expand Down
57 changes: 44 additions & 13 deletions packages/insomnia/src/ui/components/base/prompt-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,33 @@ import React, {
useState,
} from 'react';

type PromptStateEnum = 'default' | 'ask' | 'done';
type PromptStateEnum = 'default' | 'ask' | 'loading' | 'done';

interface Props<T> {
className?: string;
disabled?: boolean;
confirmMessage?: string;
doneMessage?: string;
loadingMessage?: string;
tabIndex?: number;
title?: string;
fullWidth?: boolean;
onClick?: (event: MouseEvent<HTMLButtonElement>, value?: T) => void;
referToOnClickReturnValue?: boolean;
}

export const PromptButton = <T, >({
onClick,
disabled,
confirmMessage = 'Click to confirm',
doneMessage = 'Done',
loadingMessage = 'Loading',
tabIndex,
title,
className,
fullWidth = false,
children,
referToOnClickReturnValue = false,
}: PropsWithChildren<Props<T>>) => {
// Create flag to store the state value.
const [state, setState] = useState<PromptStateEnum>('default');
Expand Down Expand Up @@ -65,17 +69,32 @@ export const PromptButton = <T, >({
clearTimeout(triggerTimeout.current);
}
// Fire the click handler
onClick?.(event);
// Set the state to done (but delay a bit to not alarm user)
// using global.setTimeout to force use of the Node timeout rather than DOM timeout
doneTimeout.current = global.setTimeout(() => {
setState('done');
}, 100);
// Set a timeout to hide the confirmation
// using global.setTimeout to force use of the Node timeout rather than DOM timeout
triggerTimeout.current = global.setTimeout(() => {
setState('default');
}, 2000);
const retVal: any = onClick?.(event);
if (!referToOnClickReturnValue) {
// Set the state to done (but delay a bit to not alarm user)
// using global.setTimeout to force use of the Node timeout rather than DOM timeout
doneTimeout.current = global.setTimeout(() => {
setState('done');
}, 100);
// Set a timeout to hide the confirmation
// using global.setTimeout to force use of the Node timeout rather than DOM timeout
triggerTimeout.current = global.setTimeout(() => {
setState('default');
}, 2000);
} else {
if (retVal instanceof Promise) {
setState('loading');
retVal.then(() => {
setState('done');
}).finally(() => {
triggerTimeout.current = global.setTimeout(() => {
setState('default');
}, 1000);
});
} else {
throw new Error('onClick must return a Promise when referToOnClickReturnValue is true');
}
}
}
};

Expand All @@ -94,6 +113,7 @@ export const PromptButton = <T, >({
promptState={state}
confirmMessage={confirmMessage}
doneMessage={doneMessage}
loadingMessage={loadingMessage}
>
{children}
</PromptMessage>
Expand All @@ -105,9 +125,11 @@ interface PromptMessageProps {
promptState: PromptStateEnum;
confirmMessage?: string;
doneMessage?: string;
loadingMessage: string;
children: ReactNode;
}
const PromptMessage: FunctionComponent<PromptMessageProps> = ({ promptState, confirmMessage, doneMessage, children }) => {
const PromptMessage: FunctionComponent<PromptMessageProps> = ({ promptState, confirmMessage, doneMessage, loadingMessage, children }) => {

if (promptState === 'ask') {
return (
<span className='warning' title='Click again to confirm'>
Expand All @@ -119,6 +141,15 @@ const PromptMessage: FunctionComponent<PromptMessageProps> = ({ promptState, con
);
}

if (promptState === 'loading') {
return (
<span className='warning' title='loading'>
<i className='fa fa-spinner animate-spin' />
<span className='space-left'>{loadingMessage}</span>
</span>
);
}

if (promptState === 'done' && doneMessage) {
return <span className='space-left'>{doneMessage}</span>;
}
Expand Down
Loading

0 comments on commit bc8aa38

Please sign in to comment.