tchristofferson / Config-Updater

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

Chinese characters are even worse and ignored sections can't be a value #28

Closed Skyslycer closed 1 year ago

Skyslycer commented 1 year ago

I recently updated to 2.1-SNAPSHOT and donated, until I noticed, that ignored values aren't a thing anymore. I found this pretty useful, although it's not 100% needed.

Now to the really stressful part: Chinese characters look even worse after the update. Before: 锁定 turned into 閿佸畾 Now: 锁定 turns into é”�定

I can also see that you added the wrong version to the tests.

On a side note: Is there a way to not remove all quotation marks that aren't necessary while keeping all that are? It kind of makes the config look messy with some values wrapped with quotation marks and some not.

Really looking forward to using this in the next update again.

Config used: https://paste.skyslycer.de/8yLTi L:20

kangarko commented 1 year ago

Same here. Ignored sections broke as of the last update @tchristofferson.

My config: https://hastebin.com/share/golupemapi.yaml

Ignored sections:

public static final List<String> UNCOMMENTED_SECTIONS = Collections.unmodifiableList(Arrays.asList(
            "Channels.List",
            "Groups",
            "Warning_Points.Reset_Task.Remove",
            "Warning_Points.Sets",
            "Messages.Prefix",
            "Messages.Discord"));

I am getting the "Ignored sections must be a ConfigurationSection..." error as the "value" seems to be null in the getSection() method now.

Skyslycer commented 1 year ago

Yup, please fix this.

tchristofferson commented 1 year ago

@kangarko I pasted your config into the test config and used the same ignored sections and I didn't get the error. Does it happen all the time?

@Skyslycer I pasted the Chinese characters into the test config and it was the same after.

Skyslycer commented 1 year ago

As I said, (I'm basing on the master branch) you pasted the wrong characters in the test config. The characters you posted were originally 2 letters, but changed by ConfigUpdater to 3 letters. Try it with the 2 letters above.

Skyslycer commented 1 year ago

Hi, I need an update on this.

kangarko commented 1 year ago

@tchristofferson I apology, nope, this time I could not reproduce anymore. I am doing more testing on my end to save your time and will follow up if needed.

kangarko commented 1 year ago

@tchristofferson I found the problem with ignored section being null, indeed it is. We let users specify a map of prefixes they want, which is an ignored section. By default, this map is empty:

a

The ignored section key I set to "Prefix" and it throws an error because the section has not been setup by the user. Can you add an exemption for this please?

// Note that the ignored section map is null in the default config as well

tchristofferson commented 1 year ago

@kangarko If it is an empty section, I believe it should be 'Prefix: {}' as the value, as that represents an empty config section. Also I think the link to your config is broken, so I can't see your config.

tchristofferson commented 1 year ago

@Skyslycer I pasted 锁定 into the config, ran update, and it is the same after.

Skyslycer commented 1 year ago

Alright, I'll try it again.

kangarko commented 1 year ago

I see, it is a bit weird that both "Section: " and "Section: {}" are valid YAML values:

a

Anyways I have tons of users leaving it empty already, plus the users routinely forget to place {} after cleaning a section - could you just add an exception for this, please?

tchristofferson commented 1 year ago

@kangarko I will try and get to this at some point.

Skyslycer commented 1 year ago

I am slowly losing hope. Guess what happened last time I tried CU to test the chinese characters: https://paste.skyslycer.de/BuNpU 😂 Is it just me or is basically nothing working for me anymore since the update? Also the Chinese chars don't work either.

Skyslycer commented 1 year ago

And it happened again! bruh

tchristofferson commented 1 year ago

This issue should be fixed on the develop branch. Although I don't think this was an issue. See test testUpdateMethodToCheckIfFilesAreSameAfter. The test passes and there are chinese characters in the config.