Fix a few issues with invoking the builtin diff editor #4078
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
30350n
force-pushed
the
fix-apply-diff-builtin
branch
from
1af148f to
9fc6741
Compare
|
Re title: perhaps call this "Fix a few issues with invoking the builtin diff editor"? [Update: I realized I can just adjust the title myself. I hope you don't mind if I do. In any case, feel free to revert the change if I misunderstood something and feel free change the title in other ways that seem good to you.] Also, I have not reviewed the implementation of this or arxanas/scm-record#62 at all, but thank you for looking into this! |
|
I'm not sure if you plan on making this a non-draft PR. If you do, see https://martinvonz.github.io/jj/latest/contributing/#code-reviews. |
ilyagr
changed the title
|
Just a reminder that you will have this draft PR. I know many users are waiting for it to get merged. |
"will" -> "still"? |
Yeah, I'd also be happy to get this resolved, but I most likely won't be able to find time to give this a once over this year anymore. I see this as a POC, to properly resolved this larger changes to scm-record are required (see scm-record #62). So if someone else wants to tackle this, feel free to build on top of this ^^ |
Proof of Concept to fix #3702 and #3846.
Relies on scm-record #62.
This is pretty ugly, especially the
unreachable!()stuff, but it seems to mostly work fine. Fixing this properly would require rewriting large parts ofscm-record.Checklist
CHANGELOG.md