misc improvements #562
misc improvements #562
Conversation
cmkohnen
commented
Nov 4, 2023
•
cmkohnen
commented
- Display used style in Figure Settings initial view and use correct design pattern (Inspired by gnome-control-center)
- Fix startup with custom style (in user directory)
- Don't inject an attribute into RcParams
- Move Style class to vala and use proper getters and setters
- Annotate all functions in style_manager
- Fix parsing error triggering not found message
- Check for ubuntu only on init to save resources
- Move preview generation to style_io and use tmpfile instead of one in cache dir
- Fix selected style not updating in some cases
- Use classmethod instead of staticmethod for items
- Implement file-like-wrapper to avoid in-memory buffers
- Progressively parse columns and styles
- fix migration issues
- Use Context manager for file handling
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
src/file_io.py
Outdated
| return json.loads(file.load_bytes(None)[0].get_data()) | ||
| wrapper = FileLikeWrapper.new_for_file_readonly(file) | ||
| return json.load(wrapper) | ||
| wrapper.close() |
There was a problem hiding this comment.
Will it actually ever reach line 95 (closing the wrapper), as it typically stops the function after the return statement no?
There was a problem hiding this comment.
Yes, you are correct, my mistake. I've pushed a fix, where a contextmanager is used, taking care of closing the stream
| @staticmethod | ||
| def new(params, xdata=None, ydata=None, **kwargs): | ||
| return DataItem( | ||
| @classmethod |
There was a problem hiding this comment.
I've actually been working a lot with custom constructors at work, where I've been using class methods for the exact same purpose. Seems the better way to me, yes. Also coincidentally reminds me on what I've been working on at work.
There was a problem hiding this comment.
The main reason, was that classmethods can be inherited. Say we introduce DataItemButWithInternalHistory which doesn't need its own new method, since it has no new properties to render. using the staticmethod we would only get a DataItem whereas a classmethod will indeed get the correct type.
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
Signed-off-by: Christoph Matthias Kohnen <christoph.kohnen@disroot.org>
| for index, line in enumerate(wrapper.wrap_text()): | ||
| if index <= skip_rows: | ||
| for index, line in enumerate(wrapper.wrap_text(), -skip_rows): | ||
| if index < 0: |
There was a problem hiding this comment.
Had to look twice to see why this unusual logic is used, but it makes line 118 cleaner. So seems more robust.
