gbv / SignaturenDruck

An electron application to print shelfmarks onto labels
https://verbundwiki.gbv.de/display/VZG/SignaturenDruck+der+ThULB+Jena
Creative Commons Zero v1.0 Universal
3 stars 2 forks source link

Invalid config file gets replaced by default #36

Closed jsubhi closed 5 years ago

jsubhi commented 5 years ago

level: high If the config.json file is invalid (due to an editing error) it gets replaced by the default config without any warning. This should not happen! Instead the Program should quit with an appropriate error message (preferably giving a hint about what is wrong with the file).

EliDeh commented 5 years ago

the config.json file only get replaced if the key 'defaultDownloadPath' can not be found.

jsubhi commented 5 years ago

The file is always replaced if the json is invalid. e.g. removing the comma after "sortByPPN": false, will trigger replacement of the file upon next start of the app. Orr adding an extra backslash to the regex: This is OK: "regex": "^([A-Z]) (.):(.)$", This will trigger replacement: "regex": "^([A-Z]) (.):(.)$",

Not a big issue but irritating if I don't have a backup of the original config since any small typo will erase my data.

jsubhi commented 5 years ago

Ich habe dies jetzt mal as bug hochgestuft. Ich habe gerade über eine halbe Stunde am Telefon verbracht, weil ich einer Kollegin helfen wollte, die Probleme mit der Konfiguration hatte. Irgendwo in der config.json war ein Fehler - wir wussten aber nicht wo. Nach jedem erfolglosen Korrekturversuch wurde unsere config.json durch die Defaultversion ersetzt sodass wir wieder unsere Version ins Verzeichnis kopieren mussten. Zusätzlich tauchen dann immer auch die default Formate auf, und das Programm beschwert sich wegen fehlender Drucker... Fatal ist auch, dass man gar nicht mitbekommt, wenn die Datei ersetzt wird - man denkt, man hat eine Änderung gemacht, z.B. den Default Mode, die hat aber keinen Effekt, weil sie gleich überschrieben wurde.

Ich möchte sehr darum bitten, dass die config.json nur dann überschrieben wird, wenn die Datei gar nicht existiert. Andernfalls sollte das Programm einfach mit einer möglichst hilfreichen Fehlermeldung abbrechen. (Sorry für den Rant, aber ich hatte durch dieses Verhalten des Programms einen sehr irritierenden Vormittag...)

jsubhi commented 5 years ago

Much better now! Personally I would prefer to just have the error message and maybe an option in that dialogue to create a clean config file - otherwise I have to rename the file in addition to fixing my type. (e.g. "Error in config.json. Do you want to create a clean sample config file? The current file will be saved as config_invalid.json" YES / NO). But that's a minor thing. Issue can be closed.

EliDeh commented 5 years ago

Thank you for your feedback. The clean config is needed to start the program. Your proposed dialogue would either keep the config as it is and close the program, because it can not start without a valid config. Or rename the current config and create a new default config and proceed. That dialogue feels like an unnecessary step to find the problem. I think its better to compare both configs and in doubt edit the new config step by step (if the problem isn't obvious).

jsubhi commented 5 years ago

The dialogue is good now. I wanted to keep the original config.json because in the case of an error I have to rename the file back to config.json in addition to fixing my error. But that would be a rare event in normal life since one would not continually mess around with config.json as I was doing during testing. So I will close this issue as it is really not that important and I can live fine with the current solution. Thanks!