Qt uses a signed value to represent indices. We should follow this
convention where applicable to avoid unnecessary sign-conversion
warnings, as well as making it easier to interoperate with other aspects
of Qt.
While we're at it, we can also make a sign-conversion explicit.
critical() is intended for critical/fatal errors that threaten the
overall stability of an application. A user entering a conflicting key
sequence is neither of those.
1. This is something that should be solely emitted by the hotkey dialog
itself
2. This is functionally unused, given there's nothing listening for the
signal.
The previous code was all "smushed" together wasn't really grouped
together that well.
This spaces things out and separates them by relation to one another,
making it easier to visually parse the individual sections of code that
make up the constructor.
Uses a std::string_view instead of a std::string, given the pointed to
string isn't modified and is only used in a formatting operation.
This is nice because a few usages directly supply a string literal to
the function, allowing these usages to otherwise not heap allocate,
unlike the std::string overloads.
While we're at it, we can combine the address formatting into a single
formatting call.
nullptr was being returned in the error case, which, at a glance may
seem perfectly OK... until you realize that std::string has the
invariant that it may not be constructed from a null pointer. This
means that if this error case was ever hit, then the application would
most likely crash from a thrown exception in std::string's constructor.
Instead, we can change the function to return an optional value,
indicating if a failure occurred.
Makes the parameter ordering consistent, and also makes the filename
parameter a std::string. A std::string would be constructed anyways with
the previous code, as IOFile's only constructor with a filepath is one
taking a std::string.
We can also make WriteStringToFile's string parameter utilize a
std::string_view for the string, making use of our previous changes to
IOFile.
We don't need to force the usage of a std::string here, and can instead
use a std::string_view, which allows writing out other forms of strings
(e.g. C-style strings) without any unnecessary heap allocations.
This allows for forming comment nodes without making unnecessary copies
of the std::string instance.
e.g. previously:
Comment(fmt::format("Base address is c[0x{:x}][0x{:x}]",
cbuf->GetIndex(), cbuf_offset));
Would result in a copy of the string being created, as CommentNode()
takes a std::string by value (a const ref passed to a value parameter
results in a copy).
Now, only one instance of the string is ever moved around. (fmt::format
returns a std::string, and since it's returned from a function by value,
this is a prvalue (which can be treated like an rvalue), so it's moved
into Comment's string parameter), we then move it into the CommentNode
constructor, which then moves the string into its member variable).
Amends cases where we were using things that were indirectly being
satisfied through other headers. This way, if those headers change and
eliminate dependencies on other headers in the future, we don't have
cascading compilation errors.
Previously, the code was accumulating data into a std::vector and then
tossing all of it away if a setting was disabled.
Instead, we can just check if it's disabled and do no work at all if
possible. If it's enabled, then we can append to the vector and
allocate.
Unlikely to impact usage much, but it is slightly less sloppy with
resources.
A few of the aoc service stubs/implementations weren't fully popping all
of the parameters passed to them. This ensures that all parameters are
popped and, at minimum, logged out.
Given the array is a private static array, we can just make it
internally linked to hide it from external code. This also allows us to
remove an inclusion within the header.
