feat: Alt hotkeys [Feedback wanted] #5875
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -7,20 +7,32 @@ | |||
| namespace chatterino { | |||
|
|
|||
| Hotkey::Hotkey(HotkeyCategory category, QKeySequence keySequence, | |||
| QString action, std::vector<QString> arguments, QString name) | |||
| QKeySequence *keySequenceAlt, QString action, | |||
There was a problem hiding this comment.
warning: pass by value and use std::move [modernize-pass-by-value]
src/controllers/hotkeys/Hotkey.cpp:1:
+
+ #include <utility>src/controllers/hotkeys/Hotkey.cpp:14:
- , action_(action)
+ , action_(std::move(action))| @@ -7,20 +7,32 @@ | |||
| namespace chatterino { | |||
|
|
|||
| Hotkey::Hotkey(HotkeyCategory category, QKeySequence keySequence, | |||
| QString action, std::vector<QString> arguments, QString name) | |||
| QKeySequence *keySequenceAlt, QString action, | |||
| std::vector<QString> arguments, QString name) | |||
There was a problem hiding this comment.
warning: pass by value and use std::move [modernize-pass-by-value]
src/controllers/hotkeys/Hotkey.cpp:15:
- , arguments_(arguments)
+ , arguments_(std::move(arguments))| @@ -7,20 +7,32 @@ | |||
| namespace chatterino { | |||
|
|
|||
| Hotkey::Hotkey(HotkeyCategory category, QKeySequence keySequence, | |||
| QString action, std::vector<QString> arguments, QString name) | |||
| QKeySequence *keySequenceAlt, QString action, | |||
| std::vector<QString> arguments, QString name) | |||
There was a problem hiding this comment.
warning: pass by value and use std::move [modernize-pass-by-value]
src/controllers/hotkeys/Hotkey.cpp:16:
- , name_(name)
+ , name_(std::move(name))| @@ -560,7 +554,8 @@ void HotkeyController::clearRemovedDefaults() | |||
| void HotkeyController::tryAddDefault(std::set<QString> &addedHotkeys, | |||
| HotkeyCategory category, | |||
| QKeySequence keySequence, QString action, | |||
| std::vector<QString> args, QString name) | |||
| std::vector<QString> args, QString name, | |||
There was a problem hiding this comment.
warning: the parameter 'args' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
src/controllers/hotkeys/HotkeyController.hpp:131:
- std::vector<QString> args, QString name,
+ const std::vector<QString>& args, QString name,| std::vector<QString> args, QString name, | |
| const std::vector<QString>& args, QString name, |
| @@ -560,7 +554,8 @@ | |||
| void HotkeyController::tryAddDefault(std::set<QString> &addedHotkeys, | |||
| HotkeyCategory category, | |||
| QKeySequence keySequence, QString action, | |||
| std::vector<QString> args, QString name) | |||
| std::vector<QString> args, QString name, | |||
There was a problem hiding this comment.
warning: the parameter 'name' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
src/controllers/hotkeys/HotkeyController.hpp:131:
- std::vector<QString> args, QString name,
+ std::vector<QString> args, const QString& name,| std::vector<QString> args, QString name, | |
| std::vector<QString> args, const QString& name, |
|
nitpick nothing to do with the code: user-facing references to alternative/alternate should not be shortened, since |
apa420
changed the title
|
Is there a need to have infinite keybinds per hotkey action or is just 2 enough? |
#5872
I would appreciate feedback on this (not on the code rn). This would basically add the possiblity of a alternate keybind to an already existing action. This would make creating more keybinds for the same action easier and also allow us to set multiple keybinds for the same default action more intuitavely (see shift+enter and ctrl+shift+enter).