littlerobots / version-catalog-update-plugin

Gradle plugin for updating a project version catalog
Apache License 2.0
565 stars 23 forks source link

allow keeping the original line breaks/formatting in the TOML #112

Open FyiurAmron opened 1 year ago

FyiurAmron commented 1 year ago

With big files, splitting into sections and using vertical space increases readability. The comment nodes are respected and left alone, so they can be used as a workaround, but would it be possible to just allow keeping the line breaks etc. also (as a config param probably)?

I guess that current approach of storing comments per key could be extended to store newlines as "comments" of a kind also? data class Comments looks like it could easily handle just \n in its Strings, e.g.

[libraries]
# newline below

# after the newline
spring-boot-starter = { module = "org.springframework.boot:spring-boot-starter" }

would then make spring-boot-starter store 3 comments, [ "# newline below", "\n", "# after the newline" ]

It could be handled just as another case in https://github.com/littlerobots/version-catalog-update-plugin/blob/main/catalog/src/main/kotlin/nl/littlerobots/vcu/VersionCatalogParser.kt#L60 , along the lines of

line.trim().isEmpty() && keepEmpty -> currentComment.add(line)
hvisser commented 1 year ago

I'm not sure about this. The comment situation is already a bit tricky (especially with sorting enabled) because they are not parsed by the toml parser itself. The comment can be seen as an annotation to an entry, while for whitespace this feels a but more fuzzy. As you mention this can be worked around now by using (empty) comments in stead of line breaks, or the whitespace removal can be reverted after updating the catalog when using vcs.

FyiurAmron commented 1 year ago

@hvisser yeah, I get your point. From a "pure" perspective, implementing what I proposed directly would be a code abuse. Still, processing of metadata (including comments, whitespace, newlines, annotations etc.) in config files was always a tricky problem, and there is no real answer here (see e.g. https://github.com/marzer/tomlplusplus/issues/28 , https://github.com/go-yaml/yaml/issues/709 or the famous problem of JSON comments) - for a "purer" approach, renaming the xxxComments vars etc. to e.g. xxxMetadata or something like that would be better I guess. Comments are metadata after all, and whitespace and formatting around the data entry is also metadata in this context. AFAIR, if something is neither a comment or part of a data entry, it has to be whitespace/newline, as per https://github.com/toml-lang/toml/blob/1.0.0/toml.abnf

;; Overall Structure

toml = expression *( newline expression )

expression =  ws [ comment ]
expression =/ ws keyval ws [ comment ]
expression =/ ws table ws [ comment ]

Even the https://toml.io/en/v1.0.0 doc uses newlines to pad the files extensively, especially before [tables] and to separate two different dotted string groups: (quote from doc, literal)

# RECOMMENDED

apple.type = "fruit"
apple.skin = "thin"
apple.color = "red"

orange.type = "fruit"
orange.skin = "thick"
orange.color = "orange"

Anyway, if this functionality was hidden behind a boolean config flag that would be false by default, no real harm would be done anyway, right?

Raenar4k commented 1 year ago

Thanks for making this cool plugin :)

I would also want this feature. Even replacing newlines with # it still deletes lines containing compose_compiler_version which is used in project.

IMO any formatting is extra, and it would be much better if tool could only change version numbers without changing anything else.

image
hvisser commented 1 year ago

That is unrelated to formatting, you should mark versions not used in dependencies with @keep in a comment for that. The version is being removed because it is considered unused.

Raenar4k commented 1 year ago

Thanks for update! With this it works almost perfectly. Sorry if this is wrong issue for discussing this, but there is one more unexpected issue: Plugin changes this:

mockk = { module = "io.mockk:mockk", version.ref = "mockk_version" }
kotlin-reflect = { module = "org.jetbrains.kotlin:kotlin-reflect", version.ref = "kotlin_version" }

to

mockk = { module = "io.mockk:mockk", version.ref = "mockk_version" }
kotlin-reflect = "org.jetbrains.kotlin:kotlin-reflect:1.9.0"

Which is used like this:

    testImplementation(libs.mockk)
    testImplementation(libs.kotlin.reflect)
hvisser commented 1 year ago

@Raenar4k yes, this is unrelated, please file a new issue. It could be that one of your dependencies is pulling in kotlin-reflect-1.9.0 which is different from your kotlin_version.

Raenar4k commented 1 year ago

Alright, thanks! But back at topic.

It would be nice to be able to use line breaks to make toml more readable and not have them deleted each time we run plugin. Im using workaround with comments for line breaks for now.