Comment 4 for bug 1662035

Revision history for this message
Patrick Storz (ede123) wrote :

Here's a first version of a patch for this issue.

The diff is a bit bulky as I did some reformatting of constructor signatures plus some automatic whitespace fixes (sorry about that), however changes in functionality should be minimal:
- Most importantly every "Parameter" now has an "_indent" member variable (specifying the indentation level; set in "Parameter::make()" (see parameter.cpp)
- Indentation is achieved by using "set_margin_left()" on the parameter's widget. There are only two occurrences (one in prefdialog.cpp; the other in notebook.cpp) unless I missed anything.

The most visible changes which I hope to discuss:
- Previously the "indent" attribute often had no effect at all, e.g. for notebooks but also for parameters with 'appearance="full"'. These conditionals are not (yet) implemented but I'm unsure if I should actually add them back since I find this behavior not only unpredictable but also unnecessary: Why not let the extension author decide freely which parameters to indent? This removes a lot of complexity and makes it much easier to use the attribute. In fact in all cases where internal extensions specify indentation it has no effect (don't know if this was intentional...).
- "description"s always used an indentation level of at least one (even if no indentation was specified). While this might make sense in some cases (where descriptions are used to explain parameters) I guess it's also mostly unexpected among extension authors. In my opinion "description" parameters should behave like all other parameters. After all case mentioned before would be the perfect use case for indentation and I guess there are many cases where a default indentation would be undesirable.

Let me know what you think and if/what I should change in the patch!