High-level goals:
- Decouple the external interface from the implementation better.
  Perhaps reduce the external interface to a single #include.
- Make the option parser code planner-independent.

Some details:
- Split the code into smaller parts with clearly defined responsibilities.
- Reduce internal dependencies in option parser code.

General observations about the code that are not yet converted to issues;
some of these may already have been addressed by the changes in the week of
September 17-21, 2018:

- Currently, some of the users of the option parser use the files in the
  main source directory, and some of them directly use the files in the
  "options" directory. The header files in the main source directory are
  marked as temporary stubs, but we think that perhaps they could be more
  generally useful to separate the public interface of the option parser
  from its internals. Whether or not we keep the stubs, all user code should
  access the option parser in the same way.

- The separation between a parser and a parse was very non-existent/unclean.
  This is now quite a bit better by reducing global object use, but there
  is probably still a lot more that can be done.

- The parse_tree.h file and more generally the use of the external tree
  library should be rethought. We are perhaps better off not using an external
  tree implementation.

- The actual parsing of text is currently fragile and should perhaps be
  reimplemented. Perhaps it can also be separated out more from the other
  code.

- The files option_parser.{h,cc} are very large and have multiple
  responsibilities. (This has become better, but may still be the case.)

- As a general principle, we want to have less code in *.h files and more
  code in *.cc files in the option parser because all code in *.h files is
  some form of internal code dependency. Perhaps it is worth using the
  pimpl idiom in a few places to reduce the amount of code that needs to
  live in header files.

- The option parser design should be documented better.

- The parsing code requires better error checking. It should not accept
  invalid specifications (e.g. using non-existent arguments), and when
  faced with an invalid specification, it should give a clean error message.

Specific issues:
- issue481: avoid special case for open lists.
- issue524: Use shared_ptr for landmark factories
- issue526: Use shared_ptr for Heuristic/Evaluator
- issue606: move the code to a directory
- issue608: split code into more files
- issue619: Get rid of need to list plugin types manually for get_help_templ
- issue626: introduce a namespace.
- issue717: Remove special cases for landmark synergy
- issue718: Avoid special case for Heuristics and ScalarEvaluators
- issue764: remove global variables for plan storage
- issue826: split option_parser.{h, cc} into smaller responsibilities
- issue827: Move code from .h to .cc files
- issue830: Use single registry
- issue831: unique names for plug-ins and predefinitions
- issue832: Rework relation Registry-DocPrinter
- issue834: Remove traces of old Plugin class
- issue836: Don't use global object for predefinitions
- issue840: Don't use global object for registry
- issue842: Two-phase initialization of registry (first collect plugin info,
  then construct them later) with better name collision reporting
- issue844: avoid special case for plugin types that support predefinition
- issue873: Make the option parser code planner-independent
- issue507: make keywords mandatory

From what I can tell, this is all done, and hence I'm marking it as resolved.

Going through the things listed in the summary, I don't see a concrete on-going TODO item that hasn't been addressed. If you think there is something there that we haven't addressed and that should still be addressed, please speak up.

Add issue507 to list.

> Metaclasses are under discussion for C++20

Updated word is they definitely won't be added before C++23 because there is
still too much work to do.

>> Is there interest in working on this further?
>
> I think it would improve the usability of the planner a lot if we made the
> parser more rigorous. In any case, we should decide on which headers to use
> for the option parser to get out of this state of limbo :-)

Sounds good!

>> Should we do this within this issue, or call this one done and open a
>> new one with a clean slate?
>
> I don't have a preference either way.

Nobody else expressed a preference either, so as a tie breaker let's apply the
principle of doing what causes the least amount of immediate work and keep this
issue as the one where we keep track of further option parser refactoring
activities.

So the next step is to identify the next steps. Jendrik mentions two things
above: making the parser more rigorous, and unifying the #includes. I've added
the first item to the summary; the second was already included.

Answering to the first part of msg8365 (will answer to the rest separately):

> One more thing that is currently a bit complicated for users of the option
> parser is the inheritance of options. Ideally, I think a subclass should
> automatically inherit all options from the parent, but I don't know how hard
> it would be to achieve this or if it's worth looking into this.

This is not generally possible to do in C++ in a DRY way. It would require a
language feature like classmethods, introspection into the set of base classes,
or metaclasses. Metaclasses are under discussion for C++20, but it's one of
these big language changes (like concepts) where it's not yet clear if, when and
in which form it will be accepted. For those who are interested, check out Herb
Sutter's recent talks on metaclasses and reflection, for example this one:

    https://www.youtube.com/watch?v=80BZxujhY38

(Note that he talks about an unrelated topic first.)

If we don't worry too much about DRY and are willing to accept that the factory
function has to repeat the base class in some way, then we could make it
possible, but not with the current plugin design because there is nothing
visible outside the factory function that knows which kind of object is actually
created. That is, the factory functions for SumEvaluator, FFHeuristic and
LandmarkCountHeuristic all have the same interface from the perspective of the
option parser: they return a shared_ptr<Evaluator>, and there is no way to
figure out what kind of evaluator it is.

The easiest and cleanest way I can think of to make "inheritance within plugins"
a thing is to move away from our current plug-in design, which is essentially
just wrapped factory functions, to more explicit factory classes. This has the
added advantage that things that are currently mixed up in our factory functions
could be separated. For example, we could get rid of all the "if dry_mode" tests.

We would still need to repeat ourselves: for FFHeuristic to inherit the options
of Heuristics, we would need to have a class FFHeuristicPlugin deriving from a
class HeuristicPlugin, and FFHeuristicPlugin::add_options (or whatever) would
still have to call HeuristicPlugin::add_options manually, or we need some kind
of magical base-class and then derive FFHeuristicPlugin from
MagicBaseClass<FFHeuristic, Heuristic>. Overall, we would probably end up with
more code per plug-in rather than less, but the code could perhaps be cleaner
than it currently is.

There are other ways to design this. Using RTTI, another way to reduce
boilerplate is to explicitly tell the option parser that FFHeuristic derives
from Heuristic and then register each plugin together with the ID of its
associated type. But many of the designs that require less user code end up with
a lot of magic that has its own disadvantages.

In summary, I think this is a major change that would be interesting to work on,
but I wouldn't see it as part of "refactoring" but rather as a larger new
feature request. If we want to work on it, I'd do it outside this issue.

One more thing that is currently a bit complicated for users of the option
parser is the inheritance of options. Ideally, I think a subclass should
automatically inherit all options from the parent, but I don't know how hard it
would be to achieve this or if it's worth looking into this.

> Is there interest in working on this further?

I think it would improve the usability of the planner a lot if we made the
parser more rigorous. In any case, we should decide on which headers to use for
the option parser to get out of this state of limbo :-) 

> Should we do this within this issue, or call this one done and open a new one
> with a clean slate?

I don't have a preference either way.

I have cleaned up the summary a bit.

All specific issues we have opened are addressed now, which raises the question
how to proceed. It would be nice to improve the code further now that we have
some momentum. I think now would be a good time to take a step back, look at the
option parser code as a whole again and identify what should be the next steps
in improving the option parser codebase.

There are certainly things that are still very ugly. On the user side, the most
visible things are the lack of proper error checking in the parsing code, the
ugliness required in the iterated search code, and the fact that we currently
use two different sets of header files to access the option parser. On the code
side, I would need to take stock first to identify what the worst bits are at
the moment, but certainly the actual parsing code must be up there on the list
of things in need of improvement.

Is there interest in working on this further?

Should we do this within this issue, or call this one done and open a new one
with a clean slate?

Add issue873 to summary.

added missing issue840

I added some thoughts from the current sprint to the summary in section "General
observations about [...]".

Summary: added reference to issue844 and minor cleanup.

Moved items in summary with specific issue numbers from "Some details" to
"Specific issues".

Clean-up of summary: be consistent about use of strikethrough. (Don't use manual
strike-through in the list of specific issues; instead rely on roundup's
automatic striking through of resolved issues.)

Added issue842 to summary and removed more deteailed comments from the summary
that were specific to issue842. They have been added to that issue as msg7717.

Fix summary.

Add issue834 to summary.

For issue764 we should talk about the way that options like
"--internal-plan-file" are parsed.

Update summary.

Copy plans for detecting plugin name collisions from issue729.

We should only strike these things through once the code is merged. In the
current master repository, the special case for Heuristic and Evaluator is still
present. Hence, I'm putting it back in, but adding a reference to issue718 there.

Updated summary by striking item "Avoid special case for Heuristics and
ScalarEvaluators". This item was resolved with issue718.

Testing the tracker after server maintenance.

updated summary to show that issue626 is merged.

Split off issue626 to introduce a namespace.

Attached a graph showing the current header dependencies.

Made the summary a bit more structured.

Updated summary to take into account issue608 is done.

Added issue608 for splitting the option parser code into more files.

Updated summary.

issue606 will move the code to a subdirectory.

This is a followup to issue586.

The plugin support in the option parser still has a few gaps:

- All plugin types still need to be manually listed in two places in
option_parser.cc
  (calls to get_help_templ).
- Heuristics and ScalarEvaluators need special-casing in a few places because
  of their inheritance relationship. The suggestion made in issue586 for this
  was to get rid of the plugin inheritance feature and unify Heuristics and
  ScalarEvaluators from the UI perspective.
- Open lists need special-casing because they are templated. This is issue481.
- Plugin types that support predefinition still need special-cased code, e.g. to
  define the command-line option name. Currently, the only such options are
  --heuristic for Heuristic and --landmarks for LandmarkGraph.

Additionally, there are other things in the option parser that could be refactored:
- The code should be moved to a directory and use a namespace.
- If possible, the compile-time dependencies should be reduced: currently all users
  of plugins and/or options drag in a lot of code, mainly because of the extensive
  use of templates. Perhaps we can decouple the external interface from the
  implementation better.

Ideally, we would like the option parser code to be encapsulated to the extent
that it could be reused in a different project without changes.