-
Notifications
You must be signed in to change notification settings - Fork 41
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
[WIP] Format checker for notes.json file #36
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! I just have a couple of points to address before I'm ready to merge.
if 'x' not in note or 'y' not in note: | ||
raise ValueError("No coordinate value found (x,y)") | ||
if 'height' not in note or 'width' not in note: | ||
raise ValueError("No height or width value found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really only need title, text, and color to be there. The position and size info is not used in the manager, and is auto-generated by the note if it doesn't already exist. However, we might want to make sure the structure of the file is correct (ie the root item should be a dict, with group names as keys, and the values being lists of dicts).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, should we consider value inside color, because if it's undefined color will be defaulted as gray?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's necessary. It wont cause a crash, and the only way to have an invalid color atm is to manually edit the file (in which case the user should see the issue quite quickly and know exactly how to fix it).
I made a format check while loading from the JSON file. It was written in the source code as to-do.