-
Notifications
You must be signed in to change notification settings - Fork 612
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
installer: skip Experimental Options page, if empty #578
installer: skip Experimental Options page, if empty #578
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks great. We should at some point rework the GP_
prefix recycling though to avoid confusion.
installer/install.iss
Outdated
@@ -423,6 +423,7 @@ const | |||
GP_BuiltinAddI = 4; | |||
GP_EnablePCon = 5; | |||
GP_EnableFSMonitor = 6; | |||
GP_Max = 6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great idea, but the name is slightly confusing since we use the same GP_
prefix for other things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I revamped this rather completely. Could you have another look, @rimrul?
Signed-off-by: Johannes Schindelin <[email protected]>
For drop-down and radio button options, we used to manually pick the largest value when defining the size of the corresponding array. Let's instead introduce consistent `..._Max...` constants for this purpose. This will make the next commit cleaner, where we need to iterate over all experimental options. Signed-off-by: Johannes Schindelin <[email protected]>
As of a75602a (installer: hide the "experimental" FSMonitor setting by default, 2024-07-17), the FSMonitor option is hidden from the options page unless the user had enabled it previously. However, this then left the page blank (or: all options hidden), not a good user experience. So let's skip it if it would appear blank. This fixes git-for-windows/git#5231. Signed-off-by: Johannes Schindelin <[email protected]>
…xperimental option If all of the experimental options are hidden, we skip that page. However, this is the last page of the wizard (at least if there are no active processes that would block the installation), and that is therefore the page where we re-label the "Next" button so that it says "Install". Therefore, if the experimental options page is hidden, we need to move that button re-labeling to the preceding page. Signed-off-by: Johannes Schindelin <[email protected]>
70911c3
to
18e1a8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix and nice QOL improvements too :-)
/add relnote bug Due to a bug introduced in the v2.47 cycle, the installer showed an empty "experimental options" page, which was fixed. The workflow run was started |
Due to a bug introduced in the v2.47 cycle, [the installer showed an empty "experimental options" page](git-for-windows/git#5231), which was [fixed](#578). Signed-off-by: gitforwindowshelper[bot] <[email protected]>
As of a75602a (installer: hide the "experimental" FSMonitor setting by default, 2024-07-17), the FSMonitor option is hidden from the options page unless the user had enabled it previously.
However, this then left the page blank (or: all options hidden), not a good user experience. So let's skip it if it would appear blank.
This fixes git-for-windows/git#5231.