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

Merge Editor: Checkbox Should Push Undo Stop #158864

Closed
hediet opened this issue Aug 23, 2022 · 3 comments · Fixed by #159053
Closed

Merge Editor: Checkbox Should Push Undo Stop #158864

hediet opened this issue Aug 23, 2022 · 3 comments · Fixed by #159053
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug feature-request Request for new features or functionality merge-editor verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@hediet
Copy link
Member

hediet commented Aug 23, 2022

No description provided.

@hediet hediet added bug Issue identified by VS Code Team member as probable bug merge-editor labels Aug 23, 2022
@hediet hediet added this to the August 2022 milestone Aug 23, 2022
@hediet hediet self-assigned this Aug 23, 2022
hediet added a commit that referenced this issue Aug 24, 2022
@hediet hediet linked a pull request Aug 24, 2022 that will close this issue
hediet added a commit that referenced this issue Aug 24, 2022
@connor4312
Copy link
Member

This works in the simple case but is easy to break if you check boxes on multiple sides of the editor, for example

Kapture.2022-08-25.at.16.01.55.mp4

@connor4312 connor4312 reopened this Aug 25, 2022
@connor4312 connor4312 added the verification-found Issue verification failed label Aug 25, 2022
@hediet hediet modified the milestones: August 2022, September 2022 Aug 26, 2022
@hediet hediet modified the milestones: September 2022, October 2022 Sep 29, 2022
@hediet hediet modified the milestones: October 2022, November 2022 Oct 4, 2022
@hediet hediet modified the milestones: October 2022, November 2022 Oct 24, 2022
@hediet hediet closed this as completed Nov 18, 2022
@hediet hediet added feature-request Request for new features or functionality verification-needed Verification of issue is requested and removed verification-found Issue verification failed labels Nov 28, 2022
@hediet
Copy link
Member Author

hediet commented Nov 28, 2022

Verification steps:

  • Open a merge conflict using the Open Merge Editor State From JSON command with this JSON:
{
    "languageId": "javascript",
    "base": "const { readFileSync } = require('fs');\n\nlet paths = process.argv.slice(2);\nmain(paths);\n\nfunction main(paths) {\n    // print the welcome message\n    printMessage();\n\n    let data = getLineCountInfo(paths);\n    console.log(\"Lines: \" + data.totalLineCount);\n}\n\n/**\n * Prints the welcome message\n*/\nfunction printMessage() {\n    console.log(\"Welcome To Line Counter\");\n}\n\n/**\n * @param {string[]} paths\n*/\nfunction getLineCountInfo(paths) {\n    let lineCounts = paths.map(path => ({ path, count: getLinesLength(readFileSync(path, 'utf8')) }));\n    return {\n        totalLineCount: lineCounts.reduce((acc, { count }) => acc + count, 0),\n        lineCounts,\n    };\n}\n\n/**\n * @param {string} str\n */\nfunction getLinesLength(str) {\n    return str.split('\\n').length;\n}\n",
    "input1": "const { readFileSync } = require('fs');\n\nlet paths = process.argv.slice(2);\nmain(paths);\n\nfunction main(paths) {\n    // print the welcome message\n    printMessage();\n\n    const data = getLineCountInfo(paths);\n    console.log(\"Lines: \" + data.totalLineCount);\n}\n\nfunction printMessage() {\n    console.log(\"Welcome To Line Counter\");\n}\n\n/**\n * @param {string[]} paths\n*/\nfunction getLineCountInfo(paths) {\n    let lineCounts = paths.map(path => ({ path, count: getLinesLength(readFileSync(path, 'utf8')) }));\n    return {\n        totalLineCount: lineCounts.reduce((acc, { count }) => acc + count, 0),\n        lineCounts,\n    };\n}\n\n/**\n * @param {string} str1\n */\nfunction getLinesLength(str) {\n    return str.split('\\n').length;\n}\n",
    "input2": "const { readFileSync } = require('fs');\n\nlet paths = process.argv.slice(2);\nrun(paths);\n\nfunction run(paths) {\n    // print the welcome message\n    printMessage();\n\n    const data = getLineCountInfo(paths);\n    console.log(\"Lines: \" + data.totalLineCount);\n}\n\nfunction printMessage() {\n    console.log(\"Welcome To Line Counter\");\n}\n\n/**\n * @param {string[]} paths\n*/\nfunction getLineCountInfo(paths) {\n    let lineCounts = paths.map(path => ({ path, count: getLinesLength(readFileSync(path, 'utf8')) }));\n    return {\n        totalLineCount: lineCounts.reduce((acc, { count }) => acc + count, 0),\n        lineCounts,\n    };\n}\n\n/**\n * @param {string} str2\n */\nfunction getLinesLength(str) {\n    return str.split('\\n').length;\n}\n",
    "result": "const { readFileSync } = require('fs');\n\nlet paths = process.argv.slice(2);\nrun(paths);\n\nfunction run(paths) {\n    // print the welcome message\n    printMessage();\n\n    const data = getLineCountInfo(paths);\n    console.log(\"Lines: \" + data.totalLineCount);\n}\n\nfunction printMessage() {\n    console.log(\"Welcome To Line Counter\");\n}\n\n/**\n * @param {string[]} paths\n*/\nfunction getLineCountInfo(paths) {\n    let lineCounts = paths.map(path => ({ path, count: getLinesLength(readFileSync(path, 'utf8')) }));\n    return {\n        totalLineCount: lineCounts.reduce((acc, { count }) => acc + count, 0),\n        lineCounts,\n    };\n}\n\n/**\n * @param {string} str\n */\nfunction getLinesLength(str) {\n    return str.split('\\n').length;\n}\n",
    "initialResult": "const { readFileSync } = require('fs');\n\nlet paths = process.argv.slice(2);\nrun(paths);\n\nfunction run(paths) {\n    // print the welcome message\n    printMessage();\n\n    const data = getLineCountInfo(paths);\n    console.log(\"Lines: \" + data.totalLineCount);\n}\n\nfunction printMessage() {\n    console.log(\"Welcome To Line Counter\");\n}\n\n/**\n * @param {string[]} paths\n*/\nfunction getLineCountInfo(paths) {\n    let lineCounts = paths.map(path => ({ path, count: getLinesLength(readFileSync(path, 'utf8')) }));\n    return {\n        totalLineCount: lineCounts.reduce((acc, { count }) => acc + count, 0),\n        lineCounts,\n    };\n}\n\n/**\n * @param {string} str\n */\nfunction getLinesLength(str) {\n    return str.split('\\n').length;\n}\n"
}
  • Use the code lenses to resolve merge conflicts and the ignore buttons to ignore sides.
  • Verify undo/redo works

@joyceerhl joyceerhl added the verified Verification succeeded label Nov 30, 2022
@joyceerhl
Copy link
Contributor

Maybe this is expected behavior but I found it confusing that undo only works if focus is in the result editor. If I click a codelens in Input 1 and then set focus back to Input 1, ctrl+z does nothing:
Recording 2022-11-30 at 11 10 09

@github-actions github-actions bot locked and limited conversation to collaborators Jan 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug feature-request Request for new features or functionality merge-editor verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants