Pgfkeys based options

[benjamin-weiss]

Although it is not ready to merge yet, here is my pgfopts implementation. To sum up the discussion in #90 here is how the implementation works.

Every package

Options are set up with a .is choice handler. E.g.

\pgfkeys{
  /metropolis/outer/progressbar/.cd,
    .is choice,
    none/.code={•},
    head/.code={•},
}

The main key is metropolis and each sub-package has its own subkey (inner, outer, color). Depending on if the options is an "insert-like" or and "execute-like" option each possible option value sets either a beamertemplate or calls a designated macro.

% insert-like option
none/.code=\setbeamertemplate{progress bar in head/foot}[none]
% execute-like option
none/.code=\@metropolis@sectionpage@none,

If the code for an "execute-like" option is very short the intermediate macro step is omitted and the code is executed directly.

frametitleoffset/.code=\setlength{\@metropolis@voffset}{#1},

To be able to keep most of the pgfopts code on top of each document, the default values are set in a separate macro which will be called together with \ProcessPgfPackageOptions at the end of each document.

\newcommand{\@metropolis@outer@setdefaults}{
  \pgfkeys{/metropolis/outer/.cd,
    numbering=counter,
    progressbar=none,
    frametitleoffset=2em,
  }
}

% all the package code 

\@metropolis@outer@setdefaults
\ProcessPgfPackageOptions{/metropolis/outer}

Main package

The main package contains a macro for the user to set options mid-document. It expects a list of comma separated key=value options.

\newcommand{\metropolisset}[1]{\pgfkeys{/metropolis/.cd,#1}}

The main package also must pass on unknown options to the sub-packages. This is done with the .search also handler.

\pgfkeys{/metropolis/.cd,
  .search also={
    /metropolis/inner,
    /metropolis/outer,
    /metropolis/color,
    },
}

Sadly if an option is present in multiple sub-packages, the options must be passed on manually.

block/.code=\pgfkeysalso{
  inner/block=#1,
  color/block=#1,
}

Documentation

I defined a new DescribeOption macro to get a nice layout for the options.

\DescribeOption{key}{list of possible values}{default value}{
  A short description of the option.
}

Before merging I want to add some more documentation and add a titleformat option.

During testing I noticed a bug in the colortheme. If you change the normal text color mid-document, not all the derived colors are set properly. If anyone sees a quick fix I will implement it here in this PR otherwise I will open a new issue.

So, what do you think?

[benjamin-weiss]

The case settings are now also package options and the most important parts are documented.

[rchurchley]

Great work! The changes look reasonable at first glance and I'll test it out when I get the chance.

If the code for an "execute-like" option is very short the intermediate macro step is omitted and the code is executed directly.

Might \@metropolis@colors@dark be short enough to be omitted and done inline like this?

During testing I noticed a bug in the colortheme. If you change the normal text color mid-document, not all the derived colors are set properly. If anyone sees a quick fix I will implement it here in this PR.

Have you tried adding use=normal text to the non-updating \setbeamercolors? If use does not cascade, that could be the cause of the problem.

[benjamin-weiss]

Might \@metropolis@colors@dark be short enough to be omitted and done inline like this?

Originally I wanted only to omit it if the code would fit in one 80-char line. But later I dismissed this strict rule. So now I would say, yes we can probably omit it.

Have you tried adding use=normal text to the non-updating \setbeamercolors?

I just had a quick look after I discovered this behavior and that's what I checked first. And as far as I have seen it then, they all had a use key.

[benjamin-weiss]

I had another look at the font color bug and found the problem. Some of the colors in beamercolorthemedefault.sty are defined like this.

\setbeamercolor{author}{}

And these lazy defined colors do not update after normal text is changed. So if we redefine them e.g. like

\setbeamercolor{author}{parent=titlelike}

the background option works as it should. As this is a bit of work to carefully check all the colors, I propose we do that in a different PR.

[rchurchley]

How is progress coming along on this? I've finally gotten a chance to play around with it and the only issues I've come across so far involve the progressbar option.

First (a quick fix) you use the key /metropolis/outer/progressbar/head everywhere except in the key definitions, where it is top. So invoking progressbar=top won't do anything and progressbar=head won't compile.

Second (and relatedly) I would much prefer this option value be called title or frametitle. It definitely shouldn't be top, since the progress bar is added below the frametitle. But it also shouldn't be head, since the beamer headline template is also above the frametitle. Once this is done, I intend to create a new pull request to add options for a progress bar in the headline or footline (see #46) using progressbar=head or progressbar=foot, respectively.

Finally, and relatedly to the above, I think that progressbar=frametitle should not set the progress bar in head/foot template (which I'd like to use in other places). Perhaps it could set the frametitle template, or even use \addtobeamertemplate instead?

[benjamin-weiss]

How is progress coming along on this?

No progress the last week. I was waiting for your feedback.

First (a quick fix) you use the key /metropolis/outer/progressbar/head everywhere except in the key definitions, where it is top. So invoking progressbar=top won't do anything and progressbar=head won't compile.

I renamed it in the progress and obviously broke it.

Second (and relatedly) I would much prefer this option value be called title or frame title.

Makes sense. So frametitle it is.

Perhaps it could set the frametitle template, or even use \addtobeamertemplate instead?

The \addtobeamertemplate would break a mid-document change of this option. But the frametitle template would work. So I'm fine with using it instead. Makes probably even more sense.

[rchurchley]

No progress the last week. I was waiting for your feedback.

Ah — sorry for holding up development, then!

The \addtobeamertemplate would break a mid-document change of this option. But the frametitle template would work. So I'm fine with using it instead. Makes probably even more sense.

Sounds good. Once this PR is through we can investigate options for de-duplicating the resulting code.

[benjamin-weiss]

Ah — sorry for holding up development, then!

No problem. It was actually quite good this way. Because I worked more on my report I have to write for my studies instead of distracting myself with working on the mtheme. 😃

Sounds good. Once this PR is through we can investigate options for de-duplicating the resulting code.

I committed the changes. Now everything should work how it should. I also renamed the metropolisset macro to metroset. I think it is a more convenient name. What do you mean with "de-duplicating the resulting code"?

Maybe you could look over it a last time, but I think this PR is then ready to merge.

By the way, shall we keep my demo-minimal.tex or shall I delete it?

[rchurchley]

Seems to work for me. If we've missed anything we can always fix it later!

What do you mean with "de-duplicating the resulting code"?

Since \defbeamertemplate{frametitle}{plain} and \defbeamertemplate{frametitle}{progressbar} share most of their code, it might be possible to do something with (e.g.) parent templates so we don't have to repeat ourselves. But that's for a different pull request.

By the way, shall we keep my demo-minimal.tex or shall I delete it?

Since its code is already contained in the manual, I don't see the need to keep it around.

[benjamin-weiss]

Since \defbeamertemplate{frametitle}{plain} and \defbeamertemplate{frametitle}{progressbar} share most of their code, it might be possible to do something with (e.g.) parent templates so we don't have to repeat ourselves. But that's for a different pull request.

Ok, now I get it.

@matze You can merge this PR now. Just as a reminder: this PR also closes #90 and #94 and #89 is still open but was fixed with #93.

[matze]

Thanks for the effort @benjamin-weiss.

Concerning closing issues associated with a particular PR: you can do that automatically with a prefix such as Fix #89: foo bar baz in the commit summary.