Cabal Exactprint new proposal

[leana8959]

This succeeds Jappie's proposal in #5 by documenting the multiple changes of direction during the implementation.

On a practical note: the document was written in typst and then converted to markdown using pandoc, so I could use variables to generate references at the end of the document.

Rendered: https://github.com/leana8959/cabal-proposals/blob/eb11c41ba512ed712f1a701c3261e91d59fa6bfb/proposals/cabal-exactprint.md

[Mikolaj]

Splendid. Thank you for not settling for a so-so design. Is the document standalone or does it assume the acquaintance with the two older ones or refer to them a lot?

[Bodigrim]

Swap out in fl'? But it is "new" one already, no?

[leana8959]

I agree that this explanation is hand-wavy. I split the algorithm into two cases and added implementation examples and desired outcome. Please give it a read and let me know if it is clearer. Thank you for reviewing this :)

[leana8959]

@Mikolaj

Is the document standalone or does it assume the acquaintance with the two older ones or refer to them a lot?

I wrote down all the experiments I have done in the past year in a section, and then detailed the learning for each of them in this document, so it is self contained :)

[gbaz]

My views on the open questions:

Whitespaces

Cabal allow leading spaces to be (plain whitespace) or \t (tab). We would like to know if it is possible to enforce the usage of plain whitespace across all cabal files. There is an existing todo comment to enforce the use of plain whitespace in field indentation in field lexer.

Trailing whitespaces and lines with only whitespaces are also lost in the current exactprint implementation. To restore them, they need to be saved. This would entail more modification to the lexer and field parser. We want to know if it's feasible to drop them. On a related note, git can be configured to detect trailing whitespaces and warn the user, or automatically remove them.

I think it is feasible to replace a line with only whitespace as a single blank line (i would prefer if blank lines stuck around, but would be comfortable with them being removed if necessary).

I think removing trailing whitespaces is also fine.

And I also agree that uniformly replacing tab with four spaces would be welcome, cabal already yells at us for tabs (and I think hackage rejects them), so we really shouldn't be using them.

Line endings

On a windows machine, lines are ended with CRLF instead of LF. It shouldn't be hard to detect if a cabal file uses one or the other. However, we want to discuss on what to do regarding mixed line endings.

In general I think in mixed line endings there should be an option to the pretty-printer for which to default to, which consumers (such as cabal) can then configure based on user preference or which system they are being run on. Basically, detect it in the parser so it can be inspected, and "kick the problem downstream" to consumers.

Sorting

It is possible to sort a cabal field using the proposed API. The loss of trivia is local to the field, but it has some problems:

Comments will stay where they were originally.

In-field-lines trivia will be lost.

Sorting is more of a formatter feature, which exactprint doesn't try to perfect. We want to know if the current implementation is satisfactory.

This seems fine, as long as the information is exposed to consumers. Then, as you say, the formatters can try to resolve this in their own ways.

[leana8959]

I think it is feasible to replace a line with only whitespace as a single blank line (i would prefer if blank lines stuck around, but would be comfortable with them being removed if necessary).

I think removing trailing whitespaces is also fine.

And I also agree that uniformly replacing tab with four spaces would be welcome, cabal already yells at us for tabs (and I think hackage rejects them), so we really shouldn't be using them.

Awesome. It would make the exactprint task significant easier if cabal developers can agree on this choice formally.

In general I think in mixed line endings there should be an option to the pretty-printer for which to default to, which consumers (such as cabal) can then configure based on user preference or which system they are being run on. Basically, detect it in the parser so it can be inspected, and "kick the problem downstream" to consumers.

Just to be clear, it is totally possible for the current exactprinter implementation to use either \r\n for windows or \n for unix(-like). The difficulty lies in remembering where the user uses \r\n and \n if they mixes both endings, similar to remembering tabs and spaces.

Currently the printer "prints", but is not "exact". It will use the configured lineending everywhere.

In comparison, I think it would be simpler to implement exactprint for braces. The reason being that braces is clearly attached to a field or a section so we know which Field ann node to attach it to. When modified, the modification's impact can be reasoned. When it's about lineendings or tabs/spaces, they can be anywhere and/or outnumber the amount of fields, it's less clear how we should represent them. It would surely lead to changes to the Field ann data type, or maybe we would need to create a new data type.

The idea to make the printer's lineending configurable is now implemented. It should work, but it hasn't been tested.

This seems fine, as long as the information is exposed to consumers. Then, as you say, the formatters can try to resolve this in their own ways.

Awesome, thanks for the feedback.

[jappeace]

This is the proof of concept implementation: haskell/cabal#12316

note it manages already the 99% and it's small (4.5k lines addition). that's because this approach is a lot easier todo.

[Kleidukos]

Wasn't Newtype retired in haskell/cabal@75d677f8c?

[Kleidukos]

Suggested change

	addNewDependency = addValueList @CommaVSep @Identity @Dependency Prepend
	addNewDependency = addValueList @CommaVSep @Identity @Dependency Append

[Kleidukos]

Suggested change

	(Depedency pname _ libs) | pname == mkPackageName "base" -> Just (Depedency pname targetVersion libs)
	(Dependency pname _ libs) | pname == mkPackageName "base" -> Just (Dependency pname targetVersion libs)

[Kleidukos]

Suggested change

	removeDependency = removeValueList @CommaVSep @Identity @Depedency
	removeDependency = removeValueList @CommaVSep @Identity @Dependency

[Kleidukos]

Suggested change

	-> (a -> Bool)
	=> (a -> Bool)