Merge lp://staging/~ted/ubuntu-app-launch/app-object into lp://staging/ubuntu-app-launch/16.04

A first round of remarks and comments.

On Tue, 2016-02-09 at 12:34 +0000, Thomas Voß wrote:
> + g_main_context_iteration(creationcontext, TRUE /*
> > may block */);
>
> A static constexpr const variable may_block is self-documenting.

That's in the old C code, so there is no constexpr there.

> > +#pragma once
> > +#pragma GCC visibility push(default)
>
> Would you mind adding a macros PROJECT_NAME_DLL_PUBLIC/PRIVATE that
> renders symbols/types public and private, respectively?

Frankly, that's a rather ugly solution. Here everything in a public
header is public and a private header is private. It seems like a very
clean solution without putting linker clutter into the source code.

> > +namespace Ubuntu
>
> namespace names should be lower case. A namespace is not a type and
> should not be capitalized.

Uhg, okay. I think that:

Ubuntu::AppLaunch::Application

Looks very consistent. And type names are almost the only place that
you see the namespace elements in code.

Changed r234

> > +{
> > +namespace AppLaunch
> > +{
> > +
> > +struct AppID
> > +{
> > +    struct PackageTag;
> > +    struct AppNameTag;
> > +    struct VersionTag;
> > +
> > +    typedef TypeTagger<PackageTag, std::string> Package;
> > +    typedef TypeTagger<AppNameTag, std::string> AppName;
> > +    typedef TypeTagger<VersionTag, std::string> Version;
> > +
> > +    Package package;
> > +    AppName appname;
> > +    Version version;
> > +
> > +    operator std::string() const;
>
> Please define a non-member std::ostream& operator<<(std::ostream&,
> const AppID& app_id) instead.

Why instead? We need to be able to use AppID's as strings for usage
with older API's in many places. This seemed like the best way to do
it. I'm not sure why overloading shift left logical makes sense for
that.

> > +    int operator==(const AppID& other) const;
>
> Wrong return type, should be bool. Please make the function a non-
> member function, too.
>
> > +    int operator!=(const AppID& other) const;
>
> Wrong return type, should be bool. Please make the function a non-
> member function, too.

Interestingly, I stole that prototype from a C++ book I have here that
seems to be incorrect. Unsure why it matters whether they're member
functions or not. Having them outside just seems like more typing.

r235

> > +
> > +    AppID();
> > +    AppID(Package pkg, AppName app, Version ver);
> > +    bool empty() const;
>
> I don't think we should model empty as an explicit function here (it
> evaluates if the strings are set). Alternatively, I would propose
> that instances of AppID are passed as an Optional<AppID>. With that,
> you can get rid of the specialized c'tors and make the type a true
> value type. An additional benefit is getting rid of implicit
> semantics on the strings.

I love getting rid of the implicit semantics, but I don't think we
should use experimental features like optional. And without an implicit
constructor you can't have code like:

AppID foo;

if (bar) {
   foo = foo1;
} else {
   foo = foo2;
}

Which is a pain, because the compiler wants on its first pass to make
sure that it can build foo. Even though it optimizes out the building
of foo as soon as it evaluates its lifecycle.

> > +
> > +    static...