Skip to content
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

Closes #1033: focus search bar text selects all even if focused #1035

Closed
wants to merge 3 commits into from

Conversation

mcomella
Copy link

#1033.

I'm not very familiar with C++ so I apologize for any basic mistakes. 😬

@mcomella
Copy link
Author

Build passes on ubuntu but fails on Windows: the error is:

C:\projects\zeal\src\libs\ui\mainwindow.cpp(163): error C2079: 'searchBarFocusShortcuts' uses undefined class 'std::array<QString,2>' [C:\projects\zeal\dist\src\libs\ui\Ui.vcxproj]
C:\projects\zeal\src\libs\ui\mainwindow.cpp(163): error C2440: 'initializing': cannot convert from 'initializer list' to 'int' [C:\projects\zeal\dist\src\libs\ui\Ui.vcxproj]
  C:\projects\zeal\src\libs\ui\mainwindow.cpp(163): note: The initializer contains too many elements
C:\projects\zeal\src\libs\ui\mainwindow.cpp(164): error C2228: left of '.begin' must have class/struct/union [C:\projects\zeal\dist\src\libs\ui\Ui.vcxproj]
  C:\projects\zeal\src\libs\ui\mainwindow.cpp(164): note: type is 'int'
C:\projects\zeal\src\libs\ui\mainwindow.cpp(164): error C3536: 'it': cannot be used before it is initialized [C:\projects\zeal\dist\src\libs\ui\Ui.vcxproj]
C:\projects\zeal\src\libs\ui\mainwindow.cpp(164): error C2228: left of '.end' must have class/struct/union [C:\projects\zeal\dist\src\libs\ui\Ui.vcxproj]
  C:\projects\zeal\src\libs\ui\mainwindow.cpp(164): note: type is 'int'
C:\projects\zeal\src\libs\ui\mainwindow.cpp(165): error C2100: illegal indirection [C:\projects\zeal\dist\src\libs\ui\Ui.vcxproj]

My assumption is that QString is not imported on Windows despite it being so on Linux so I'll explicitly add the QString header. While I'm at it, I realized I could deduplicate QStringLiteral by using QString::fromStdString in the loop. I think this allocates memory on the heap (it returns a copy) but I think that's fine because this string should live for the lifetime of the application.

)

Previously, the search bar focus shortcut would only select all if the
search bar was not focused. This change makes select all happen all the
time, making the keyboard shortcut consistent no matter what is focused.
@mcomella
Copy link
Author

I got the same error: I assume it's because <array> was not imported. That being said, the stdlib isn't used often in the codebase so I switched to QStringList which might play nicer with Windows.

@trollixx
Copy link
Member

Thanks for contributing, and I apologize for how long it took me to get to this. I was working on a major refactoring (#1081), and kind of ignored everything else.

While reviewing your change, and reading Qt docs, I discovered a better way to implement #1033 with the help of QEvent::ShortcutOverride events. I pushed 5ef5111, and now the search box can handle Ctrl+K/Ctrl+L on its own, and set the selection accordingly. I am closing this pull request, but thank you for bringing my attention to the issue, and please test the new behavior.

@trollixx trollixx closed this Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants