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

Inline conflicts should use standard git markers #1350

Closed
marcelmindemann opened this issue Oct 2, 2023 · 18 comments · Fixed by #1396
Closed

Inline conflicts should use standard git markers #1350

marcelmindemann opened this issue Oct 2, 2023 · 18 comments · Fixed by #1396

Comments

@marcelmindemann
Copy link

marcelmindemann commented Oct 2, 2023

Actual Situation

I am using copier 8.3.0. When updating a rendered project that diverged from the template, inline markers are added to files with conflicts. These markers are custom to copier and therefore not recognized by git or my IDE.

Desired Situation

The marker should use the standard git notation and set the status of the destination git repository to "conflicts". Similar to how https://github.com/cruft/cruft leaves the conflicting files after a failed git apply <patch>.

Proposed solution

Would this not solve many problems elegantly? It would not only get rid of the need to use pre-commit to prevent pushing conflicting files, as git would not allow commiting the working tree in "conflicting files" state. Moreover, my IDE's git integration would help me to solve conflicts visually (using, for example, the brilliant 3-way-merge tool of Jetbrains or VSCode).

@pawamoy
Copy link
Contributor

pawamoy commented Oct 2, 2023

I noticed that VSCode didn't offer the same visual help for Copier markers. Would improve the UX, I agree.

@yajo
Copy link
Member

yajo commented Oct 4, 2023

I see visual help on VSCode with conflicts generated by Copier. I can use bundled tools to accept or reject changes. Could you please be more specific on what you're missing? Maybe some video or screenshots would help.

@pawamoy
Copy link
Contributor

pawamoy commented Oct 4, 2023

The differences I noticed:

  • no "Accept incoming" or "Accept current" buttons in the editor, directly above the conflict
  • no previous/next buttons in the top bar (go to previous/next conflict)
  • no check for marker presence when staging files within the VCS tab

Colors (blue/green) are there in the editor though.

@12rambau
Copy link
Contributor

12rambau commented Oct 6, 2023

image

It's working just fine on my end.

@pawamoy
Copy link
Contributor

pawamoy commented Oct 6, 2023

Huh, OK 🤔 I'll run some experiments 🙂

@yajo yajo added triage Trying to make sure if this is valid or not and removed enhancement labels Oct 8, 2023
@yajo
Copy link
Member

yajo commented Oct 8, 2023

I see it like @12rambau. Can you guys share a screenshot of the problem?

@pawamoy
Copy link
Contributor

pawamoy commented Oct 16, 2023

No "Accept" buttons 😕

conflict

Same for every other conflict throughout the code base.

@yajo
Copy link
Member

yajo commented Oct 22, 2023

The accept buttons only appear on normal file view. If you're viewing the diff file view, it won't work. That's just normal VSCode behavior. Could you check if that's the problem?

@pawamoy
Copy link
Contributor

pawamoy commented Oct 22, 2023

You're right! I didn't know that. Here are more screenshots.


During a Git merge with conflicts:

vscodemerge

Note that I'm in the Source Control tab.


After a Copier update, if I click on the file itself:

vscodecopier1

If I click on the file in the Source Control tab, I land on the diff view:

vscodecopier2

...which makes sense, but is a different experience than after a Git merge. If you look back at the first screenshot, clicking on a file in the Source Control tab gets me to the "normal" view (with buttons), not the diff one (without buttons). And files with conflicts are grouped under "Merge Changes", not just "Changes".


Another difference, as mentioned by @marcelmindemann, is that if I stage a file with conflicts after a Copier update, VSCode does not warn me about existing conflicts in the file:

conflict_nowarn.mp4

If I do the same thing during a Git merge, VSCode warns me:

conflict_warn.mp4

So I wonder, is there something more that Copier should do after an update so that VSCode (or other editors/tools) provides the same UX as during a Git merge? Again, as @marcelmindemann stated in their first message, could we somehow "set the status of the destination git repository to conflicts"? I don't know Git enough to tell if such a thing is possible. And maybe even if it's possible, it wouldn't make sense, because a Copier update is not a Git merge (in terms of UX, that's debatable)?

Status during merge:

% git status
On branch test-merge
You have unmerged paths.
  (fix conflicts and run "git commit")
  (use "git merge --abort" to abort the merge)

Changes to be committed:
        modified:   .gitignore
        new file:   build.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
        both modified:   Makefile
        both modified:   duties.py
        both modified:   pyproject.toml

Status after update:

% git status       
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   .copier-answers.yml
        modified:   .github/ISSUE_TEMPLATE/bug_report.md
        modified:   .github/ISSUE_TEMPLATE/feature_request.md
        modified:   CONTRIBUTING.md
        modified:   Makefile
        modified:   README.md
        modified:   config/ruff.toml
        modified:   docs/credits.md
        modified:   duties.py
        deleted:    mkdocs.insiders.yml
        modified:   mkdocs.yml
        modified:   pyproject.toml
        modified:   scripts/gen_credits.py
        modified:   scripts/gen_ref_nav.py
        modified:   src/griffe/cli.py
        modified:   tests/test_cli.py

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        .github/ISSUE_TEMPLATE/config.yml
        config/vscode/
        src/griffe/debug.py

no changes added to commit (use "git add" and/or "git commit -a")

Are there files in the .git folder that could hold additional, interesting information?

@yajo
Copy link
Member

yajo commented Oct 24, 2023

even if it's possible, it wouldn't make sense, because a Copier update is not a Git merge

In reality, this is the correct answer.

Conflict statuses in Git happen in several contexts: git merge, git am, git rebase... All of those will report a different status, and VSCode reacts differently to them.

For Copier, the most similar status would be git am IMHO. But still, I don't think we should dig that deep, honestly.

@pawamoy
Copy link
Contributor

pawamoy commented Oct 24, 2023

% diff aftermerge afterupdate 
Only in aftermerge: AUTO_MERGE
Common subdirectories: aftermerge/branches and afterupdate/branches
diff aftermerge/HEAD afterupdate/HEAD
1c1
< ref: refs/heads/mypyc
---
> ref: refs/heads/main
Common subdirectories: aftermerge/hooks and afterupdate/hooks
Binary files aftermerge/index and afterupdate/index differ
Common subdirectories: aftermerge/info and afterupdate/info
Common subdirectories: aftermerge/logs and afterupdate/logs
Only in aftermerge: MERGE_HEAD
Only in aftermerge: MERGE_MODE
Only in aftermerge: MERGE_MSG
Common subdirectories: aftermerge/objects and afterupdate/objects
diff aftermerge/ORIG_HEAD afterupdate/ORIG_HEAD
1c1
< 848deb0f30014809dafd94d0762d72a45210b1bf
---
> bc4fa4e1d59133baf654fdbfb5464c72b028c6e5
Common subdirectories: aftermerge/refs and afterupdate/refs
% cat aftermerge/AUTO_MERGE                   
a467ec4456bc5b9f468319b845f005212e801f07
% cat aftermerge/MERGE_HEAD 
bc4fa4e1d59133baf654fdbfb5464c72b028c6e5
% cat aftermerge/MERGE_MODE 
% cat aftermerge/MERGE_MSG 
Merge branch 'main' into mypyc

# Conflicts:
#       .gitignore
#       Makefile
#       duties.py
#       pyproject.toml

I tried to copy such files in the .git folder after a Copier update, but that didn't improve the UX. Will maybe post on StackOverflow. Super curious to know if there's a solution.

But yeah feel free to close 🙂

@yajo
Copy link
Member

yajo commented Oct 28, 2023

We can reopen later if there's any path for improvement. 😉

@yajo yajo closed this as not planned Won't fix, can't repro, duplicate, stale Oct 28, 2023
@pawamoy
Copy link
Contributor

pawamoy commented Oct 30, 2023

@pawamoy
Copy link
Contributor

pawamoy commented Oct 30, 2023

So there is a solution! It involves running a git update-index --index-info command, with manually crafted input 🙂 To build this input, another command can be used: git ls-files --stage, in order to get the SHA of "unmerged" files (the ones containing conflict markers), but I suppose there are other ways to just get the SHA (and maybe permissions, like 100644) for each file to which Copier adds conflict markers.

To try and summarize how this could be achieved: during the update, if Copier is aware that a file has or will have conflict markers, it could obtain and store its SHA and perms (with some Git-fu). If it is not aware of that, it can wait the end of the update to obtain SHAs and perms with git ls-files --stage or other Git-fu. Then it would use these SHAs and perms to build input looking like this:

0 0000000000000000000000000000000000000000	src/dependenpy/cli.py
100644 5c40f079ef475a712eeb07dd79ca5cfb2247944a 1	src/dependenpy/cli.py
100644 5c40f079ef475a712eeb07dd79ca5cfb2247944a 2	src/dependenpy/cli.py
100644 5c40f079ef475a712eeb07dd79ca5cfb2247944a 3	src/dependenpy/cli.py
0 0000000000000000000000000000000000000000	Makefile
100644 8a1218a1024a212bb3db30becd860315f9f3ac52 1	Makefile
100644 8a1218a1024a212bb3db30becd860315f9f3ac52 2	Makefile
100644 8a1218a1024a212bb3db30becd860315f9f3ac52 3	Makefile

(you'll notice 4 lines for each file containing conflict markers)

This input would then be fed to the git update-index --index-info command.

@pawamoy
Copy link
Contributor

pawamoy commented Oct 30, 2023

To (somehow) demonstrate that it works, I've tried it in a project of mine, on a file with conflict markers, src/dependenpy/cli.py. The stage listing shows that this file now has entries with mode 1, 2, and 3, and VSCode does group the file under "Merged Changes", while taking me to the view with conflict buttons when I click on it:

indexinfo

@yajo
Copy link
Member

yajo commented Oct 31, 2023

Well, it looks like this became actionable!

@yajo yajo reopened this Oct 31, 2023
@yajo yajo added enhancement and removed triage Trying to make sure if this is valid or not labels Oct 31, 2023
@yajo yajo added this to the Community contribution milestone Oct 31, 2023
@pawamoy
Copy link
Contributor

pawamoy commented Oct 31, 2023

Does it mean you'd accept a PR?

@pawamoy
Copy link
Contributor

pawamoy commented Oct 31, 2023

While working on this, I might have spotted a bug in the code adding inline markers. It has this condition:

if not (line.startswith("?? ") and line.endswith(".rej")):
    continue

But if the file contains spaces, Git will wrap its name with double-quotes, so the line won't end with ".rej" but with ".rej\"".

Ah, that probably what the next comment is telling about: # FIXME Test with a file named 'â ñ"', see it fail, fix it.

pawamoy added a commit to pawamoy/copier that referenced this issue Oct 31, 2023
pawamoy added a commit to pawamoy/copier that referenced this issue Oct 31, 2023
pawamoy added a commit to pawamoy/copier that referenced this issue Oct 31, 2023
pawamoy added a commit to pawamoy/copier that referenced this issue Oct 31, 2023
pawamoy added a commit to pawamoy/copier that referenced this issue Nov 12, 2023
pawamoy added a commit to pawamoy/copier that referenced this issue Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants