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

Unified merge view - improve what text is in cm-deletedText #1376

Open
ondrejmirtes opened this issue Apr 30, 2024 · 6 comments
Open

Unified merge view - improve what text is in cm-deletedText #1376

ondrejmirtes opened this issue Apr 30, 2024 · 6 comments

Comments

@ondrejmirtes
Copy link

Describe the issue

Hello,
I'm really grateful that CM6 includes unifiedMergeView. When I started a project that needs it, I had no idea whether I'll have to spend several days implementing my own (inferior) solution, but fortunately I found @codemirror/merge.

I have a suggestion: in the diff like this:

Screenshot 2024-04-30 at 9 46 12

I'd like if -next-line part of the deleted line was inside cm-deletedText so that it'd also be highlighted similarly to identical.alwaysFalse, booleanAnd.alwaysFalse part of the added line.

This is currently how the DOM looks like:

<div class="cm-deletedChunk" contenteditable="false"><del>				/** @phpstan-ignore-next-line */</del><span class=" cm-deletedText"></span></div>
<div class="cm-changedLine cm-line"><ins class="cm-insertedLine">				<span class="ͼy">/** @phpstan-ignore</span><span class="cm-changedText"><span class="ͼy"> identical.alwaysFalse, booleanAnd.alwaysFalse</span></span><span class="ͼy"> */</span></ins></div>

I'd prefer if it looked something like this:

<div class="cm-deletedChunk" contenteditable="false"><del>				<span class="ͼy">/** @phpstan-ignore</span><span class=" cm-deletedText"><span class="ͼy">-next-line</span></span><span class="ͼy"> */</span></del></span></div>
<div class="cm-changedLine cm-line"><ins class="cm-insertedLine">				<span class="ͼy">/** @phpstan-ignore</span><span class="cm-changedText"><span class="ͼy"> identical.alwaysFalse, booleanAnd.alwaysFalse</span></span><span class="ͼy"> */</span></ins></div>

Thank you for considering this.

Browser and platform

No response

Reproduction link

No response

@marijnh
Copy link
Member

marijnh commented Apr 30, 2024

Deleted text is highlighted. I'm not sure how that screenshot was made, it seems to use different styling than the default (compare). Maybe some mistake was made in the custom styles?

@ondrejmirtes
Copy link
Author

Yeah, I have custom styles but without them it still doesn't work. It can't because the current DOM is not structured for that.

This is how it looks with the default styles:

Screenshot 2024-04-30 at 11 04 49

@marijnh
Copy link
Member

marijnh commented Apr 30, 2024

Oh, right, unified view, I was looking at the split merge view. That indeed doesn't seem to render changed spans separately.

@ondrejmirtes
Copy link
Author

@ondrejmirtes
Copy link
Author

One unrelated question: how can I expect getChunks(state) to look in the unified merge view? Do they always have side set to 'b'? It seems like that from my testing (I don't know what text should I produce in order to have some chunks on side 'a'). Thanks.

@marijnh
Copy link
Member

marijnh commented Apr 30, 2024

side is a per-editor thing that tells you which side of the chunks represent the document in that editor. In a split merge view both editors have a different side. In a unified view the editor is always b, and a refers to the given original document.

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

No branches or pull requests

2 participants