pablo-s / passes

Manage your digital passes
GNU General Public License v3.0
66 stars 16 forks source link

Improve file chooser #24

Closed TheEvilSkeleton closed 1 year ago

TheEvilSkeleton commented 1 year ago

I marked it as draft to encourage a discussion on how we can go about writing the code. We did not discuss it, so I'm completely okay if this MR gets closed.

This MR does the following changes:

I tested locally with GNOME Builder and I can confirm it works.

Eventually we can add application/vnd.apple.pkpasses.

Related to https://github.com/pablo-s/passes/issues/8.

pablo-s commented 1 year ago

Hello @TheEvilSkeleton. Thanks for all your help and contributions!

I have been checking and testing your changes and I would do the following changes:

  1. Instead of creating the module "supported_formats", I would do the following: 1.1. In the abstract class DigitalPass (inside module digital_pass) I would add the following class methods: supported_formats() and supportedextensions(). 1.2. Then I would implement these methods in each of the subclasses of DigitalPass, so that they return appropriate values for the pass type. 1.3. Lastly, I would implement the mentioned method also in the parent class (DigitalPass). It can dynamically collect all values from subclasses (using the method __subclasses_\), combine them and return them.

  2. When setting the name of the filter, I would include its name between _( and ) so that the text can be extracted and translated.

Would it make sense to include another entry for "All files" in the filter? In the event that we have a pass with incorrect (or even without) extension.

What do you think?

TheEvilSkeleton commented 1 year ago

My apologies, I'm new to Python so there are a few things I do not understand. What is "class method"? I looked online and this is what I got: https://www.programiz.com/python-programming/methods/built-in/classmethod. Is this the right thing I'm looking for?

pablo-s commented 1 year ago

Yes! :)

That way we can ask to the class for the supported formats:

supported_formats = DigitalPass.supported_formats()

And from within the method we have a reference to the class object that we can use:

@classmethod
def supported_formats(cls):
    for subclass in cls.__subclasses__():
        ...

That was my idea at least. In this way, I thought there is no need to modify another module when a new pass format is added in the future.

Let me know if you have any other question.

TheEvilSkeleton commented 1 year ago

I have realised that you have added the shortcut to load a pass (Control+O), but you have not included it in the shortcuts window (src/view/help_overlay.blp). It makes sense that it is also included there.

Good catch.