Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add better merge extension #27150

Merged
merged 22 commits into from
May 23, 2017
Merged

Conversation

pprice
Copy link
Contributor

@pprice pprice commented May 23, 2017

Adds the better merge extension to vscode proper. (https://marketplace.visualstudio.com/items?itemName=pprice.better-merge). This will enable decoration, code-lenses and commands for "standard" style merge conflicts.

Notable changes:

  • better-merge.* command contributions changed to merge-conflict.* with same keybindings and functionality.
  • Settings collapsed down to merge-conflict.codeLens.enabled and merge-conflict.decorators.enabled
  • Replace the single pass regex scanning of entire document, with line-by-line scanning, sans regex.
  • Added localization (package.nls.json and vcode-nls usage) support
  • Remove the git extension decoration of merge blocks.

Known TODOs:

  • Performance measurements for large files. Maximum to test up to is 5mb as anything larger is not synchronized with extensions.
  • It is possible to fire two scan consumptions from the same "origin" code, this mostly happens in mergeDecorator.ts, when a document is opened then active. The actual scan happens once as it is debounced via the delayer, BUT the merge decorators are sent to the document twice. I have a candidate fix in the works.
  • Replace hard-coded decorator colors with something to get theme colors when available.

Potential TODOs (probably future PRs)

  • Support diff3 format

@mention-bot
Copy link

@pprice, thanks for your PR! By analyzing the history of the files in this pull request, we identified @joaomoreno and @dbaeumer to be potential reviewers.

@msftclas
Copy link

@pprice,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot


const rightTitle = `Incoming changes`; // (Ln${range.start.line}${!range.isSingleLine ? `-${range.end.line}` : ''})`;

const title = `${fileName}: ${leftTitle} \u2194 ${rightTitle}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe inline the two titles and use a placeholder {0} to localize like: https:/Microsoft/vscode/blame/master/extensions/git/src/commands.ts#L754

}
],
"configuration": {
"title": "Merge Conflict",
Copy link
Contributor

Choose a reason for hiding this comment

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

Extract to nls file for completeness.

}

private getConflictsOrEmpty(document: vscode.TextDocument): interfaces.IDocumentMergeConflict[] {
let stepStart = process.hrtime();
Copy link
Contributor

Choose a reason for hiding this comment

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

Skip or remove console output.

"description": "Merge Conflict",
"version": "0.7.0",
"aiKey": "AIF-d9b70cd4-b9f9-4d70-929b-a071c400b217",
"enableProposedApi": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed, I believe. Remove the property to clarify.

*--------------------------------------------------------------------------------------------*/

/// <reference path='../../../../src/vs/vscode.d.ts'/>
/// <reference path='../../../../src/vs/vscode.proposed.d.ts'/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed, I believe. Remove to clarify.

It is possible in the open / activate event cycle that we end up doing a double push of decorator changes to the editor, as both events pending on to the same delayed operation.
@chrmarti chrmarti merged commit 7966de7 into microsoft:master May 23, 2017
@joaomoreno joaomoreno added this to the May 2017 milestone May 24, 2017
@niyazpk
Copy link

niyazpk commented Jun 16, 2017

Just noticed this feature during a merge conflict. This is just great. Thanks a bunch!!!

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants