tchristofferson / Config-Updater

Used to update files for Bukkit/Spigot API
MIT License
66 stars 9 forks source link

Comments break after map list #24

Closed kangarko closed 1 year ago

kangarko commented 1 year ago

We have the following configuration: https://pastebin.com/B8Svtv0T

After calling ConfigUpdater#update, the file is partially "corrupted" as comments past the section of the image below no longer appear.

a

They strangelly reappear again after the map list sections are over:

b

I am using your exact code for a test case. I and my users would be really thankful if you could have a minute to look into this!

-Matej

kangarko commented 1 year ago

Tagging my friend and collaborator who provided us his settings.yml if you need further information, @AdoNikeOFFICE

tchristofferson commented 1 year ago

I will take a look at this. Are you ignoring any sections?

adrianjanocko commented 1 year ago

I will take a look at this. Are you ignoring any sections?

I tried ignoring and not ignoring, but result was the same.

tchristofferson commented 1 year ago

The issue is with the KeyBuilder. I didn't think about a list of maps. The KeyBuilder believes Announcements.Permission, for example, is a valid key when it is actually the second property in the first list of maps. I will try and get a fix for this.

kangarko commented 1 year ago

Awesome! Let me know your paypal or something and I'll donate because we've been using your library for our community for quite some time now!

tchristofferson commented 1 year ago

Fixed with pull request #25. Use dependency version 2.1-SNAPSHOT as shown in readme. If there are any issues related to this please open this issue again. If it is a different issue create a new issue.

tchristofferson commented 1 year ago

@kangarko Didn't see your comment. My donation link on PayPal is https://www.paypal.com/paypalme/tchristoffers0n. You don't need to donate if you don't want to.

kangarko commented 1 year ago

I love it! Thank you <3

@AdoNikeOFFICE can you please test on your software?

kangarko commented 1 year ago

@tchristofferson I tried to boot your update with ChatControl (settings.yml > https://hastebin.com/share/irogerabol.yaml) and I am receiving the following issue.

It looks like ignoring a section which contains subsections where keys of the map of the ignored subsection are non-strings causes the problem.

Screenshot 2023-03-28 at 11 03 26

I attempted to change the (String s : map.keySet()) to "Object s" and then call toString but that results in another error: "java.lang.IllegalArgumentException: Invalid ignored section: Warning_Points.Sets.global.3"

adrianjanocko commented 1 year ago

I also tested it and this is the result of settings.yml: https://pastebin.com/nGuNDbMZ. This is the original file: https://pastebin.com/verG1Wni. Some things weren't even saved at all, some things got shuffled around, and some comments are missing.

tchristofferson commented 1 year ago

What version of Spigot is being used?

kangarko commented 1 year ago

We need it to work under the latest one (Paper/1.19.4) as well as Paper 1.8.8 for compatibility reasons. I can of course fork it and adjust for legacy versions afterwards if you don't have time.

kangarko commented 1 year ago

In the above image, I used Paper/1.8.8. It ships with an outdated YAML library but it was not a problem before.

adrianjanocko commented 1 year ago

What version of Spigot is being used?

I tested it on 1.19.2.

tchristofferson commented 1 year ago

I am looking into a solution using eo-yaml library so then I don't need to worry about formatting since Yaml is very dependent on formatting and is very nuanced.

kangarko commented 1 year ago

The biggest downside is that we'd have to shade all classes of that library into our own library or have users rely on it which I am trying to avoid. Yours is literally just one class. It would be miles better just to keep things under 1 class and fix this one particular issue.

tchristofferson commented 1 year ago

@kangarko I will shade eo-yaml so it is part of com.tchristofferson.configupdater. I get that it would be nice to keep it small, however, the code is hard to read, very sensitive to any changes, and there are probably cases where it won't work such as this case. You can keep using v2.0-SNAPSHOT which has been pretty stable and modify your yaml file so there aren't lists of maps.

tchristofferson commented 1 year ago

Try using 2.1-SNAPSHOT and let me know if it works. Please get the latest 2.1-SNAPSHOT if you already tried to use it, or use the code in the develop branch.

kangarko commented 1 year ago

Thanks! @AdoNikeOFFICE can you test please?

kangarko commented 1 year ago

@AdoNikeOFFICE I assume Christoffer meant on this branch: https://github.com/tchristofferson/Config-Updater/commits/develop

adrianjanocko commented 1 year ago

Tested with those commits and got similar or the same result: https://pastebin.com/wAyxUJN8.

tchristofferson commented 1 year ago

I committed some more code to fix this. The tests are passing so I think it should be good. Can you give this another go?

adrianjanocko commented 1 year ago

I committed some more code to fix this. The tests are passing so I think it should be good. Can you give this another go?

I am going to give it a try in a minute.

adrianjanocko commented 1 year ago

@tchristofferson, this is the result: https://pastebin.com/pDfm3zXC and this is the original file: https://pastebin.com/VuCYwpEC.

You can already see progress there, but in the middle of the file it still messes up those comments for some reason.

kangarko commented 1 year ago

@AdoNikeOFFICE This is invalid:

:): ☺

You need to change this to

":)": "☺"

Can you retry after you changed all of the Emoji keys? (Also see if there are any other sections of your settings with : in the key name, you need to put "" around it).

adrianjanocko commented 1 year ago

@AdoNikeOFFICE This is invalid:

:): ☺

You need to change this to

":)": "☺"

Can you retry after you changed all of the Emoji keys? (Also see if there are any other sections of your settings with : in the key name, you need to put "" around it).

See the original file, :): ☺ is changed like that when saving.

tchristofferson commented 1 year ago

I think it is fixed now. I used your entire config and the test passes. Let me know if its fixed. Update 2.1-SNAPSHOT or use the develop branch again to test it.

adrianjanocko commented 1 year ago

I confirm, it works! 🎉

Thank you for your willingness.

kangarko commented 1 year ago

Thanks everyone!