Fix append extension to work when ok button clicked #9945
Conversation
Fix notepad-plus-plus#9939 In some cases, checkbox was found instead of OK button and window procedure was overriden for it. Now, the OK button is identified using style and label checks. Also, in some cases IFileDialog::GetFileTypeIndex() returns the old value. To fix that, always remember the file type selected.
| } | ||
| _dialog->SetFileTypeIndex(newFileType); |
There was a problem hiding this comment.
Should there be a sanity check just to make sure that newFileType is not the same as _currentType? That is, that it actually changed?
There was a problem hiding this comment.
Good point.
But I believe, it could never happen.
This function is called each time the checkbox is clicked.
_lastSelectedType could be anything but _wildcardType (condition at the line 325).
And when the new file type is selected (user clicks an item in the dropdown list) the checkbox is checked if it was unchecked.
| @@ -51,6 +50,7 @@ namespace // anonymous | |||
|
|
|||
| static const int IDC_FILE_CUSTOM_CHECKBOX = 4; | |||
| static const int IDC_FILE_TYPE_CHECKBOX = IDC_FILE_CUSTOM_CHECKBOX + 1; | |||
| static const TCHAR TEMP_OK_BUTTON_LABEL[] = _T("Save"); | |||
There was a problem hiding this comment.
Following C++14+, this should probably be static constexpr const....
There was a problem hiding this comment.
In this context, I think it doesn't matter.
The compiler generates the same assembly.
If I were to declare this as a static member of a class, then it makes more sense.
Since I could initialize constexpr in place while for a static const I need an initializer in the .cpp.
There was a problem hiding this comment.
Sure, I just make it a habit nowadays to use constexpr wherever it is relevant instead of just const alone.
Many things will generate the same machine code output, but will have slightly different semantics, but in general I keep it constexpr because it encourages me to use it more often.
| @@ -826,7 +830,7 @@ class CustomFileDialog::Impl | |||
|
|
|||
| /////////////////////////////////////////////////////////////////////////////// | |||
|
|
|||
| CustomFileDialog::CustomFileDialog(HWND hwnd) : _impl{std::make_unique<Impl>()} | |||
| CustomFileDialog::CustomFileDialog(HWND hwnd) : _impl{ std::make_unique<Impl>() } | |||
There was a problem hiding this comment.
Strangely, MSVC (GCC and Clang do not) generates different code depending on if you brace or parantheses-initialize. Also, generates different code if you use new instead of std::make_unique.
There was a problem hiding this comment.
I don't see any significant differences between braces vs parentheses init. Below is a diff of your example when I left only int overload and only float overload:
As for new, yes, it would be different as expected.
But still, using make_unique over bare new is a good practice to avoid leaks if constructor throws:
https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique
There was a problem hiding this comment.
I don't disagree, I was just surprised at the difference as I'd expected the optimizer to reduce them to the same thing in the end.
Don't look at the wrapper_types as they are __forceinlined - look at the outputs for func0/1/2.
|
I also checked this PR on Windows 7 VM. Seems to be working fine. |
Fix #9939
In some cases, checkbox was found instead of OK button and window
procedure was overriden for it. Now, the OK button is identified
using style and label checks.
Also, in some cases IFileDialog::GetFileTypeIndex() returns
the old value. To fix that, always remember the file type selected.