Skip to content

Move app::Key to its own key.cpp file#5407

Merged
dacap merged 1 commit into
aseprite:mainfrom
dacap:move-key-definition
Sep 2, 2025
Merged

Move app::Key to its own key.cpp file#5407
dacap merged 1 commit into
aseprite:mainfrom
dacap:move-key-definition

Conversation

@dacap

@dacap dacap commented Sep 2, 2025

Copy link
Copy Markdown
Member

No description provided.

@dacap dacap self-assigned this Sep 2, 2025

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/app/ui/key.cpp Outdated
app::KeyContext context;
};

static std::vector<KeyShortcutAction> g_actions;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: 'g_actions' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

Suggested change
static std::vector<KeyShortcutAction> g_actions;
std::vector<KeyShortcutAction> g_actions;

Comment thread src/app/ui/key.cpp Outdated

static std::vector<KeyShortcutAction> g_actions;

static const std::vector<KeyShortcutAction>& actions()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: 'actions' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

Suggested change
static const std::vector<KeyShortcutAction>& actions()
const std::vector<KeyShortcutAction>& actions()

Comment thread src/app/ui/key.cpp
static struct {
const char* name;
app::KeyContext context;
} g_contexts[] = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: 'g_contexts' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

src/app/ui/key.cpp:125:

- static struct {
+ struct {

Comment thread src/app/ui/key.cpp Outdated
Vec vector;
};

static std::vector<KeyShortcutWheelAction> g_wheel_actions;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: 'g_wheel_actions' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

Suggested change
static std::vector<KeyShortcutWheelAction> g_wheel_actions;
std::vector<KeyShortcutWheelAction> g_wheel_actions;

Comment thread src/app/ui/key.cpp Outdated

static std::vector<KeyShortcutWheelAction> g_wheel_actions;

static const std::vector<KeyShortcutWheelAction>& wheel_actions()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: 'wheel_actions' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

Suggested change
static const std::vector<KeyShortcutWheelAction>& wheel_actions()
const std::vector<KeyShortcutWheelAction>& wheel_actions()

Comment thread src/app/ui/key.cpp Outdated
template<>
std::string convert_to(const app::WheelAction& from)
{
int c = int(from);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: variable 'c' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int c = int(from);
int const c = int(from);

Comment thread src/app/ui/key.cpp Outdated
case KeyAction::SquareAspect:
case KeyAction::DrawFromCenter:
case KeyAction::RotateShape: m_keycontext = KeyContext::ShapeTool; break;
case KeyAction::LeftMouseButton: m_keycontext = KeyContext::Any; break;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: switch has 2 consecutive identical branches [bugprone-branch-clone]

    case KeyAction::LeftMouseButton:           m_keycontext = KeyContext::Any; break;
    ^
Additional context

src/app/ui/key.cpp:373: last of these clones ends here

    case KeyAction::RightMouseButton:          m_keycontext = KeyContext::Any; break;
                                                                                    ^

Comment thread src/app/ui/key.cpp
}
}

const ui::Shortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: method 'isPressed' can be made static [readability-convert-member-functions-to-static]

src/app/ui/key.h:126:

-   const ui::Shortcut* isPressed(const ui::Message* msg, KeyContext keyContext) const;
+   static const ui::Shortcut* isPressed(const ui::Message* msg, KeyContext keyContext) ;
Suggested change
const ui::Shortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const
const ui::Shortcut* Key::isPressed(const Message* msg, const KeyContext keyContext)

Comment thread src/app/ui/key.cpp

const ui::Shortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const
{
if (const auto* keyMsg = dynamic_cast<const KeyMessage*>(msg)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: repeated branch body in conditional chain [bugprone-branch-clone]

  if (const auto* keyMsg = dynamic_cast<const KeyMessage*>(msg)) {
                                                                 ^
Additional context

src/app/ui/key.cpp:454: end of the original

  }
   ^

src/app/ui/key.cpp:455: clone 1 starts here

  else if (const auto* mouseMsg = dynamic_cast<const MouseMessage*>(msg)) {
                                                                          ^

Comment thread src/app/ui/key.cpp
{
switch (keyctx) {
case KeyContext::Any: return std::string();
case KeyContext::Normal: return I18N_KEY(key_context_normal);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: switch has 10 consecutive identical branches [bugprone-branch-clone]

    case KeyContext::Normal:               return I18N_KEY(key_context_normal);
    ^
Additional context

src/app/ui/key.cpp:591: last of these clones ends here

    case KeyContext::Transformation:       return I18N_KEY(key_context_transformation);
        ^

@dacap dacap force-pushed the move-key-definition branch from 0af057e to 21ad78c Compare September 2, 2025 18:23

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/app/ui/key.cpp

const ui::Shortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const
{
if (const auto* keyMsg = dynamic_cast<const KeyMessage*>(msg)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: repeated branch body in conditional chain [bugprone-branch-clone]

  if (const auto* keyMsg = dynamic_cast<const KeyMessage*>(msg)) {
                                                                 ^
Additional context

src/app/ui/key.cpp:450: end of the original

  }
   ^

src/app/ui/key.cpp:451: clone 1 starts here

  else if (const auto* mouseMsg = dynamic_cast<const MouseMessage*>(msg)) {
                                                                          ^

Comment thread src/app/ui/key.cpp
{
switch (keyctx) {
case KeyContext::Any: return {};
case KeyContext::Normal: return I18N_KEY(key_context_normal);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: switch has 10 consecutive identical branches [bugprone-branch-clone]

    case KeyContext::Normal:               return I18N_KEY(key_context_normal);
    ^
Additional context

src/app/ui/key.cpp:587: last of these clones ends here

    case KeyContext::Transformation:       return I18N_KEY(key_context_transformation);
        ^

@dacap dacap merged commit 21ad78c into aseprite:main Sep 2, 2025
12 checks passed
@dacap dacap deleted the move-key-definition branch September 4, 2025 13:52
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