fix(Jump List): Use unique name for recent repos #12144
Conversation
src/app/GitCommands/UserRepositoryHistory/RepositoryDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
src/app/GitCommands/UserRepositoryHistory/RepositoryDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
| @@ -65,6 +77,34 @@ public string Get(string repositoryDir) | |||
| return dirInfo.Name; | |||
| } | |||
|
|
|||
| public string GetDescriptiveUnique(string repositoryDir) | |||
| { | |||
| string unique = GetUnique(repositoryDir); | |||
There was a problem hiding this comment.
I mostly have unique names of my superrepos, with the worktree name (often short like ge_1, ge_2 etc). This is mostly kept here, just with a suffix (I guess I get used to that).
If a supermodule has a description, it should be used for the unique part.
If no description, then the important part is often deeper in the path, I see many that keep structures like gitclones/project1, gitclones/project2. This will then keep the unique part just visible in the tooltip so the main benefit is just being unique.
There was a problem hiding this comment.
OK, except the name for submodules. Will use some before approving.
There was a problem hiding this comment.
so the main benefit is just being unique.
That is what this PR is about.
If a supermodule has a description, it should be used for the unique part.
This should be applied to RepositoryDescriptionProvider.Get and not only if there is a description file. Then the application title would benefit from this, too.
I think of "submodule - parentmodule (branch) - Git Extensions".
In the Jump List it would appear as "submodule - parentmodule (D_path_to_parent_subfolder_submodule)".
I would add just one level of super module.
Or else:
Half the list will now be like "ICSharp" for me.
Perhaps we should add submodules to the GE-internal MRU list only, but not to the Jump List.
There was a problem hiding this comment.
so the main benefit is just being unique.
That is what this PR is about.
There are many unique "ICSharp" that cannot be differentiated in a simple way as the diff is far right.
Just one "ICSharp" is more a feature for me than in general, not objecting.
If a supermodule has a description, it should be used for the unique part.
This should be applied to
RepositoryDescriptionProvider.Getand not only if there is a description file. Then the application title would benefit from this, too. I think of "submodule - parentmodule (branch) - Git Extensions". In the Jump List it would appear as "submodule - parentmodule (D_path_to_parent_subfolder_submodule)". I would add just one level of super module.
Yes, that seem fine.
Supermodule instead of parent? (all levels will be too much)
It will not handle (slightly worse even) the the multiple clones, you cannot get it all.
Half the list will now be like "ICSharp" for me.
Perhaps we should add submodules to the GE-internal MRU list only, but not to the Jump List.
Exclude from both if implemented.
I have considered this for some years, but seen it as a setting that would be for primarily myself.
There was a problem hiding this comment.
I have considered this for some years, but seen it as a setting that would be for primarily myself.
Interested by that also : I've started to implement it long ago but there are some edge cases on currently opened submodule repo a little hacky to fix 🫤
9fce793 to
f76cc0a
Compare
| return shortName.Length == 0 ? unique : $"{shortName} ({unique})"; | ||
| } | ||
|
|
||
| public string GetUnique(string repositoryDir) |
There was a problem hiding this comment.
for future use (I have nothing in mind)
There was a problem hiding this comment.
suggest to remove from public api
| @@ -107,6 +107,7 @@ public void AddToRecent(string workingDir) | |||
|
|
|||
| // sanitise | |||
| StringBuilder sb = new(repositoryDescription); | |||
| sb.Replace(@":\", "_"); | |||
There was a problem hiding this comment.
discussion:
It would be nize to have / instead of _ in the name, but this is a filename
Similar, I would like to replace \wsl$ with /wsl or so but very minor diff anyway
| return shortName.Length == 0 ? unique : $"{shortName} ({unique})"; | ||
| } | ||
|
|
||
| public string GetUnique(string repositoryDir) |
There was a problem hiding this comment.
suggest to remove from public api
review comment: YAGNI!
Fixes #10845
Proposed changes
WindowsJumpListManager/RepositoryDescriptionProvider: Use unique name for recent reposdescriptive name with full path appended
Screenshots
Before
with
repo.gitextcontainingD:\Downloads\super\submodule2\repo\or
D:\Downloads\super\submodule1\repo\After
with
.git/descriptioncontaining:Test methodology
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.