Skip to content

Conversation

theclickman
Copy link

It is my suggestion for fixing Bug#5683 I opened months ago. git-for-windows/git#5683

It will enable preserving Windows Integration in any context of update - manually (it will check the option automatically if detected) or via silent update (winget or other path) by detecting if the Registry entries already exists and match the current installation at that time.

It is my suggestion for fising Bug 5683 I opened months ago. git-for-windows/git#5683

It will enable preserving Windows Integration in any context of update - manually (it will check the option automatically if detected) or via silent update (winget or other path) by detecting if the Registry entries already exists and match the current installation at that time. 

Signed-off-by: Nicolas DENIS <[email protected]>
@theclickman
Copy link
Author

Confirmed success. Downloaded the artifact from the generated package by the Pull Request automatic built and apply to my laptop using the same command line used to test internally - silent update. (from version 2.50.0)
Windows integration was preserved after the update completed.

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add check over Windows Integration to already exists during upgrade

Even though I tried for several minutes to parse this message, and I am still failing. What is this supposed to tell the reader?

It is my suggestion for fising Bug 5683 I opened months ago. git-for-windows/git#5683

This might help you understand what this commit is about. Leaving everybody else puzzled. So please follow the guidance in https://github.blog/2022-06-30-write-better-commits-build-better-projects/ to improve it, in particular with a strong focus on this part:

  What you’re doing Why you’re doing it
High-level (strategic) Intent (what does this accomplish?) Context (why does the code do what it does now?)
Low-level (tactical) Implementation (what did you do to accomplish your goal?) Justification (why is this change being made?)

It will enable preserving Windows Integration in any context of update - manually (it will check the option automatically if detected) or via silent update (winget or other path) by detecting if the Registry entries already exists and match the current installation at that time.

Again, I have a really tough time trying to understand what you are saying.

Signed-off-by: Nicolas DENIS [email protected]

If you don't want to provide your real email address, it is not a real Signed-off-by: footer. The purpose of including the email address in the DCO is so that you can be contacted in case there are questions about your changes.

Apart from the commit message, it strikes me as a work-around that addresses one tiny fraction of the problem that winget updates fail to preserve the user's choices. Two of those choices, to be precise. All the other user choices are missed.

Maybe a more fruitful approach would be to find out why the registry setting Inno Setup: Selected Components in Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\Git_is1 is not respected.

Comment on lines +1923 to +1937
// Git Bash Here
if RegQueryStringValue(RootKey,
'SOFTWARE\Classes\Directory\shell\git_shell\command', '', Cmd) then
begin
if Pos(AppPath, Cmd) > 0 then
WizardSelectComponents('ext\shellhere');
end;
// Git GUI Here
if RegQueryStringValue(RootKey,
'SOFTWARE\Classes\Directory\shell\git_gui\command', '', Cmd) then
begin
if Pos(AppPath, Cmd) > 0 then
WizardSelectComponents('ext\guihere');
end;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this might address your problem, it now opens up Git for Windows to more complaints along the lines "I upgraded via winget, and the Explorer integration was kept but all my other custom configuration is gone!".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My apologizes - I am not usually doing public code fixes like that. So I am surely not aware of every details of the process. Will try to do better next time.

That said - I believe you did get exactly my intention - to solve the issue of Windows Integration being lost as part of update - regardless of the path we elect to do it (winget is among them, but it in fact apply to any silent, automated update process at the end).

But I do agree also that my fix is - indeed - insufficient. I did focus on my own issue - but not the whole picture.

If I dig a bit, it could be linked to the missing UsePreviousSetupType directive in the [Setup] section to tell the setup to actually reuse the component. I will give it a try as soon as I can.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will give it a try as soon as I can.

That sounds great!

@theclickman theclickman deleted the patch-1 branch September 10, 2025 00:16
@theclickman
Copy link
Author

Cancelled and replace with Pull Request #641

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

Successfully merging this pull request may close these issues.

2 participants