Mir

Code review comment for lp:~andreas-pokorny/mir/send-input-device-state-event

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> Nits:
>
> +#include "mir/events/event_builders.h"
>
> this isn't needed by the header (or any of the files that include it).
>
> ~~~~
>
> + /**
> + * Set all pressed scan codes.
> + * \param scan_codes currently pressed
> + */
> + virtual void set_key_state(std::vector<uint32_t> const& scan_codes) = 0;
> + /**
> + * Set button state of a pointing device.
> + * \param buttons mask of the buttons currently pressed
> + */
> + virtual void set_pointer_state(MirPointerButtons buttons) = 0;
>
> These names seem a little odd for an InputSink. (Maybe it is just my
> experience with C# and Java suggesting "set_" implies these are properties.)
> Anyway, "key_state()" or "key_state_is()" would be better than
> "set_key_state()"
>
> ~~~~
>
> - std::mutex guard;
> + mutable std::mutex guard;
>
> We put the cv-qualifier after the type:
>
> std::mutex mutable guard;
>
> ~~~~
>
> +#include "mir/event_printer.h"
>
> Needed?

Nah all of that is valid, also fixed what kdub complained about. If the MP lands, I am going to prepare that in a new MP

« Back to merge proposal