fix: present removed submodules #12436
Conversation
| @@ -0,0 +1,6 @@ | |||
| namespace GitExtensions.Extensibility.Git; | |||
|
|
|||
| public interface IObjectGitItem : INamedGitItem | |||
There was a problem hiding this comment.
What's "object git item"? Would ITypedGitItem work better?
There was a problem hiding this comment.
GitItem <- IObjectGitItem <-INamedGitItem <-IGitItem
I want the "object fields": https://git-scm.com/docs/git-ls-files#_field_names
size, stage has some usages, but no implementation right now
There was a problem hiding this comment.
This is a non blocking comment, more like an exploratory discussion.
Do we need to create this hierarchy?
I know it'd be a breaking change (from an API perspective, but in reality, probably won't be significant?), any reason not to add the new properties to the INamedGitItem?
There was a problem hiding this comment.
Because INamedGitItems are used in other situations (GitRefs), where ObjectType is not relevant. It could sure be set as Unknown there
There was a problem hiding this comment.
I pushed changes to use INamedGitItem.
See src/app/GitCommands/Git/GitRef.cs for the limitation. objecttype for refs/ is not obvious
Could be retrieved with git cat-file -t refs/tags/v2.51.05 tag - annotated or git for-each-ref --format="%(objecttype)" refs/tags/v2.51.RC1 commit - lightweight
This could be implemented (or the throw in the testbranch is kept)
There was a problem hiding this comment.
Can this and #12437 be merged?
Yes, since we have agreed to accept the minor API change.
If I get a second approval
Please!
I will merge and we could merge master to release/6.0
Then I will prepare the RC.
There was a problem hiding this comment.
Pinging @gitextensions/git-extensions-team again
This and some other are blocking a RC for no reason for over two weeks now, for something that we agreed on. Please review
Some others here: https://github.com/gitextensions/gitextensions/milestone/78
There was a problem hiding this comment.
I'm on vacation, traveling. Sorry, unable to help at the moment.
There was a problem hiding this comment.
My question wasn't blocking. I trust your judgement.
There was a problem hiding this comment.
I still would like to have a second approval, with or without a1d057c
71c963a to
6a08d1a
Compare
24aedb9 to
4bda4d7
Compare
96e6268 to
c88939a
Compare
|
Rebased on #12421 to resolve conflicts (these should be merged just after each other) |
8c39c66 to
3023dff
Compare
- Submodule presentation in the file tree and diff did not present removed submodules. - In diffs, detect if the path is a submodule also if the path does not exist.
3023dff to
bf1fcc0
Compare
Proposed changes
Follow up to #12421 (comment)
Based on #12421
present removed submodules
Submodule presentation in the file tree and diff did not
present removed submodules.
In diffs, detect if the path is a submodule also if the path
does not exist.
To present submodules as diff or in file tree the submodule must be present.
By default is only the path and the commit sha known.
Previously these paths were in some scenarios presented as files, which raised error messages.
Screenshots
see also See #12421 for prev
removed submodule in commit, folder not available in worktree, .gitmodule contains the file
removed submodule, folder exists
added submodule, not in worktree
remove submodule is still like
Test methodology
manual
Merge strategy
I agree that the maintainer squash merge this PR (if the commit message is clear).
✒️ I contribute this code under The Developer Certificate of Origin.