EssentialGG / Vigilance

Configuration Utility using Elementa
GNU Lesser General Public License v3.0
54 stars 12 forks source link

Vigilant: Make sure all folders in the path exist #77

Closed Sychic closed 1 year ago

Sychic commented 1 year ago

This assumes that an empty file is a valid config file, which might be true for the default format but isn't true in general. If you look at the default onFileNotFound handler, you'll see it calls c.initEmptyFile to initialize the file.

As far as I can tell, there doesn't seem to be anything happening in c.initEmptyFile (the default impl is empty and the toml format does not override it. Though I concede that it is better to have it than not.

Also, why catch FileAlreadyExistsException of createDirectories? According to the docs that exception is only thrown from that method when a file with the same name as the directory already exists, which is generally fatal and so we really don't want to silently discard that.

Ah, I must've misread the javadoc. I thought it would throw if the directory already exists.

Also, why catch FileAlreadyExistsException at all? The file shouldn't exist at this point and if it does, something's wrong and we probably don't want to silently discard that fact (original implementation doesn't either).

That's a bit of a brain fart on my part, forgot that the file should not exist already.