Skip to content

Conversation

@ojob
Copy link

@ojob ojob commented Nov 10, 2017

This works by file selection using the GUI, or by file addition with a
drag&drop operation.

Signed-off-by: Joël Bourgault [email protected]

Joël BOURGAULT added 2 commits November 10, 2017 10:26
This works by file selection using the GUI, or by file addition with a
drag&drop operation.

Signed-off-by: Joël Bourgault <[email protected]>
Signed-off-by: Joël BOURGAULT <[email protected]>
@ojob
Copy link
Author

ojob commented Nov 10, 2017

These I201 / I202 errors are really annoying, particularly when last commit for Photocollage v1.4.4 raised no complaints, and when I made no change to these lines. RRh.

Copy link
Owner

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested it and it works good!

However I would prefer last_visited_dir and folder to default to None, and not calling Gtk.FileChooserDialog.set_current_folder() when we don't know what dir to show (i.e. on first use). This way, "recent files" would be shown instead, which is pretty handy. What do you think?

About commits: please don't use GitHub-specific things in commit titles (like issues numbers or usernames), it's hard to understand when reading emails, in git log or when rebasing. Also this project uses Linux commit messages style for consistency and readability: this would give something like "gtkgui: Remember last visited directory" (feel free to read the commit log for examples!)

PREVIEW_MAX_SIZE = 256

def __init__(self, **kw):
folder = kw.pop('folder', '.')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following would be more Pythonic and more readable, what do you think?

    def __init__(self, folder='.', **kw):
        super(PreviewFileChooserDialog, self).__init__(**kw)

        self.set_current_folder(folder)

Joël Bourgault added 3 commits November 12, 2017 18:30
Also, changed signature of PreviewFileChooserDialog, to have `folder`
as a visible parameter.

Signed-off-by: Joël Bourgault <[email protected]>
@ojob
Copy link
Author

ojob commented Nov 12, 2017

Comments appreciated and agreed! I made the changes. (Well, almost all: may I avoid rebase-ing the commits to update the wordings?)

I also added the persistance of output folder selection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants