Code review comment for lp:~mir-team/mir/cursor-spike-phase-4-implement-api

A lot of nits:

222 +MirCursorConfiguration::MirCursorConfiguration(char const* s_name)
223 +{
224 + s_name ? name = std::string(s_name) : name = std::string();
225 +}

MirCursorConfiguration::MirCursorConfiguration(char const* name) :
    name{name ? std::string(s_name) : std::string()}
{
}

~~~~

235 + std::unique_ptr<MirCursorConfiguration> c(new MirCursorConfiguration(name));
236
237 - return c;
238 + return c.release();

return new MirCursorConfiguration(name);

~~~~

263 +

Why?

~~~~

434 + auto observer = std::make_shared<UpdateCursorOnSurfaceChanges>(cursor_controller);

auto const?

~~~~

494 + std::shared_ptr<mi::Surface> top_surface_at_point = nullptr;

std::shared_ptr<mi::Surface> top_surface_at_point;

~~~~

523 + try
524 + {
525 + input_targets->remove_observer(observer);
526 + }
527 + catch (...)
528 + {
529 + }

I don't expect input_targets->remove_observer(observer) to throw.

If removing observer does throw then surely things are so broken that we should stop execution. Not eat the exception!

~~~~

730 #include "mir_test_framework/display_server_test_fixture.h"

Unused. (I suspect a whole bunch more includes are unused - e.g. #include "mir/compositor/scene.h")

~~~~

1017 +struct DeferredInProcessServer : mtf::InProcessServer
1018 +{
1019 + void SetUp() override { /* TODO this is a nasty frig around the unfortunate coupling in InProcessServer */ }
1020 + void start_server() { mtf::InProcessServer::SetUp(); }
1021 +};

You might like to update to match test_client_input.cpp (or even put somewhere common).

struct DeferredInProcessServer : testing::Test, private mtf::ServerRunner
{
    void TearDown() override { ServerRunner::stop_server(); }

    using ServerRunner::start_server;
    using ServerRunner::new_connection;
};

~~~~

2001 + StubInputSurface surface1{geom::Rectangle{geom::Point{geom::X{0}, geom::Y{0}},
2002 + geom::Size{geom::Width{1}, geom::Height{1}}},
2003 + image};
2004 + StubInputSurface surface2{geom::Rectangle{geom::Point{geom::X{0}, geom::Y{0}},
2005 + geom::Size{geom::Width{1}, geom::Height{1}}},
2006 + image};

Alignment is wrong. (That's why I don't like aligning to the open brace.)

Also, I'm pretty sure most of those explicit constructions are unnecessary.