Simplify indentation

[flip111]

The indentation system has the feeling that it could use a makeover. There are some problems with it in my opinion

1. The 4 spaces are hardcoded, when it could be an option to the prettyprinter
2. Just inheriting from PrettyPrinterAbstract is not enough, because the logic is tied there as well https://github.com/nikic/PHP-Parser/blob/master/lib/PhpParser/PrettyPrinterAbstract.php#L658-L674
3. The way it works now with a single variable on the printer level is very confusing when you want to prepare a string to be put somewhere else. For example if you first format to string the lines of the inner scope with $this->nl and then put those in the outer scope, obviously they are not magically adjusting to that parent scope, since $this->nl is just a string in the end. This requires some real discipline and some deeper understanding to create your own prettyprinter. You have to keep state in your head that is set in some other scope and the system gets pretty complicated this way. There is nothing warning you that your indent/outdent is not balanced. Generic methods like ->pStmts are also giving you some indents "for free", and it doesn't tell you in the comments of the function. I tried making a pretty printer 3 years ago, gave up for this reason. Now i am trying a new pretty printer and same problem.

[nikic]

The 4 spaces are hardcoded, when it could be an option to the prettyprinter

I'm okay with making that an option. (That is, make the number of spaces configurable, I have no interest in support for tab indentation.)

Just inheriting from PrettyPrinterAbstract is not enough, because the logic is tied there as well https://github.com/nikic/PHP-Parser/blob/master/lib/PhpParser/PrettyPrinterAbstract.php#L658-L674

I'm not sure I understand the issue there. If you extend indent/outdent to use a different indentation size, it's not clear to me why that would have an effect on the linked code.

The way it works now with a single variable on the printer level is very confusing when you want to prepare a string to be put somewhere else. For example if you first format to string the lines of the inner scope with $this->nl and then put those in the outer scope, obviously they are not magically adjusting to that parent scope, since $this->nl is just a string in the end. This requires some real discipline and some deeper understanding to create your own prettyprinter. You have to keep state in your head that is set in some other scope and the system gets pretty complicated this way. There is nothing warning you that your indent/outdent is not balanced. Generic methods like ->pStmts are also giving you some indents "for free", and it doesn't tell you in the comments of the function. I tried making a pretty printer 3 years ago, gave up for this reason. Now i am trying a new pretty printer and same problem.

Right. The pretty printer used to work by simply indenting nested code by replacing newlines with indented newlines, and having a marker to skip indentation for certain lines. That seems to be in line with what you have in mind, if I understand correctly.

That mechanism went away when support for preserving formatting was introduced. Handling indentation while preserving formatting turned out to be really hard and it took me quite a while to get this correct. You need to deal with code moving around between positions at different indentation levels, while mixing in newly generated code with matching indentation, and handling degenerate cases like code having negative indentation.

If there's a simpler way to achieve all the objectives (which would be roughly equivalent to = a simpler way that passes all existing tests), then that would be great. I just don't know what it would look like.

[flip111]

I'm not sure I understand the issue there. If you extend indent/outdent to use a different indentation size, it's not clear to me why that would have an effect on the linked code.

Wanted to change indentation to use strings instead of repeating a space.

Handling indentation while preserving formatting turned out to be really hard and it took me quite a while to get this correct.

Can you give some more details on this problem? In particular i don't understand with "newly generated code with matching indentation" ... When you have badly formatted code as input (maybe mixed tabs and spaces) how are you determining that the newly code is matching?

[elvismdev]

+1 I'm interested in the same thing, either in being able to specify tab indentation as an option or in preserving the original file indentation format, in case the indentations are tabs.

[pmjones]

Some here may be interested in the alternative pretty-printer implementation provided by PHP-Styler. The indent-string is configurable; if it is "\t", the nominal width of the tab is also configurable, to help with line-length calculations. Cf. https://github.com/pmjones/php-styler#configuration.