Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep comments in toml file #18

Closed
eugene-krivobokov opened this issue Jan 29, 2022 · 14 comments
Closed

Keep comments in toml file #18

eugene-krivobokov opened this issue Jan 29, 2022 · 14 comments

Comments

@eugene-krivobokov
Copy link

Environment:

nl.littlerobots.version-catalog-update: 0.2.1
com.github.ben-manes.versions: 0.41.0
Gradle: 7.3.2

libs.versions.toml:

[versions]
# An important explanation of choosing this version
externalDependency = "1.0-fix"

Scenario:

./gradlew versionCatalogUpdate --no-configuration-cache

Result:

libs.versions.toml:

[versions]
externalDependency = "<the latest version here>"

Expected behavior:

The plugin can't know about semantics but shouldn't erase it from the config.
Here is the source of truth for versions. This is why we keep knowledge about versions here.

libs.versions.toml:

[versions]
# An important explanation of choosing this version
externalDependency = "<the latest version here>"
@hvisser
Copy link
Contributor

hvisser commented Jan 29, 2022

This was reported in #1 already, and currently the only workaround would be to restore the comment from version history as the parser does not record comments and with sorting/reordering I don't think there would be a good way to keep the comments with the correct entry (the comment could potentially mark a whole section of things).

@eugene-krivobokov
Copy link
Author

It's a pity. It seems a blocker for us because we have loads of comments as described in expected behavior.
I'd like to use it in automation with minimal human interaction.

As I understand, this might add some clarifications toml-lang/toml#836 for the problem.
Preserving comments looks technically possible but needs to be done in a parser.

@eugene-krivobokov
Copy link
Author

the comment could potentially mark a whole section of things

I understand. It's impossible to understand such semantics without supporting in toml format.
I was thinking specifically about the case without reordering. It might be a reasonable compromise.

@hvisser
Copy link
Contributor

hvisser commented Jan 30, 2022

Yes, I understand your use case, it makes sense. One other area for comments could be providing some hints to the plugin itself, so I would like to be able to somehow parse them.

One approach I'm thinking of trying is using jackson (the parser used for toml now) in streaming mode in combination with manually marking/parsing comment sections to keep track of them. Something that could be worth to prototype...
I did some quick testing to see if streaming mode could work, but unfortunately there's no location info from Jackson available + it consumes the input buffer in one go so there's no real "current" position in streaming mode for toml it seems.

@Strawely
Copy link

Strawely commented Feb 3, 2022

@hvisser how about something like tokenization and put in VersionCatalog class during parsing several service types like comments and blank lines?

@hvisser
Copy link
Contributor

hvisser commented Feb 3, 2022

I think what I'll try is to do a second pass over the toml file because the Jackson parser does not provide any integration point to get to the tokens being parsed, I'll leave this open for now :)

@emartynov
Copy link

emartynov commented Feb 3, 2022

I see there is a toml parser that supports comments. Other idea is to combine parser to generate a new file and restore comments from the original file by hands(Hugo already wrote about it in the message above).

While searching also found comparison of different toml parsers in kotlin.

@hvisser
Copy link
Contributor

hvisser commented Feb 3, 2022

Thanks for that, I've checked out a few and they are either not compliant with the current toml spec and fail on parsing the syntax Gradle is using, or they skip comments. I did a quick check on ktoml and it seems that they also skip comments (and that project seems to be in an early state).

I'll try the second pass route first, it might not be that bad as I thought initially given the restricted syntax of the Gradle toml file.

@tprochazka
Copy link

I would also appreciate mode when this plugin will keep exactly the same toml file, just update versions inside of it.
Because I have also dependencies split into several groups like project dependencies and testing dependencies.

@tprochazka
Copy link

tprochazka commented Feb 17, 2022

It also automatically convert
{ module = "androidx.annotation:annotation", version="1.1.0" }
to
"androidx.annotation:annotation:1.3.0"
But I would like to keep the version always as a separate attribute for clarity.
IDE highlight version, so it is then much easier to found version.

@hvisser
Copy link
Contributor

hvisser commented Feb 17, 2022

I would also appreciate mode when this plugin will keep exactly the same toml file, just update versions inside of it.

That's very unlikely to happen, not only for technical reasons, but also because one of the goals is to have consistent toml file.

The formatting serves the same purpose as other formatters: it keeps the file consistent and it prevents bike shedding discussions on teams. There are multiple ways you can write the same thing in a (Gradle) toml file and the reasoning now is to pick the simplest syntax possible, just like described in the Gradle docs.

@tprochazka
Copy link

tprochazka commented Feb 17, 2022

Yes. I definitely want to keep exactly the same format for all items, it is the reason why I want to keep versions separated as an extra parameter and don't mix two different possibilities how to write it.

Simply:

androidxRecyclerview = { module = "androidx.recyclerview:recyclerview", version="1.2.1" }
androidxRoomCompiler = { module = "androidx.room:room-compiler", version.ref="androidxRoom" }

Yes. it is not the simplest way but definitely uniform across the whole file.

@hvisser
Copy link
Contributor

hvisser commented Feb 17, 2022

As I tried to point out the plugin is opinionated towards using the most simple syntax possible per Gradle docs, so I have no plans to change this.

Let's also keep this issue on the topic of retaining comments in the toml file.

@chris-hatton
Copy link

+1, thrilled to discover this plugin but I also use many comments in the TOML file which I wouldn't want to lose.
Often these are URL links to dependency project pages for developer education.

hvisser added a commit that referenced this issue May 13, 2022
Do a crude pass for locating comments and write them out to the toml file.
Comments are associated with toml table declarations and keys within the tables, so they can move around and be removed if a key or a whole table is removed.

Fixes #18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants