Added ability to import multiple files

[nickleus27]

#2403
@ice0 This pull request allows users of the gui to select and import multiple files at once from the import dialog. I overloaded the dialog_open_file with this:
App::dialog_open_file(const std::string &title, std::vector<std::string> &filenames, std::string preference)
which adds the the parameters that is a string vector. This PR adds a lot of lines of code since I overloaded the original function. I tried making it work a bool parameter, but to be honest I was a little confused how to make it work. I know I can figure out how to make it work if need be to make maintaining code easier.

[rodolforg]

#2403 @ice0 This pull request allows users of the gui to select and import multiple files at once from the import dialog. I overloaded the dialog_open_file with this: App::dialog_open_file(const std::string &title, std::vector<std::string> &filenames, std::string preference) which adds the the parameters that is a string vector. This PR adds a lot of lines of code since I overloaded the original function. I tried making it work a bool parameter, but to be honest I was a little confused how to make it work. I know I can figure out how to make it work if need be to make maintaining code easier.

Hi, nickleus27! Please (re?)read the second approach proposed by ice0 here: #2403 (comment)
That overload you did that copy'n paste almost everything is really bad for maintaining as ice0 and you already stated. His proposal is very good IMO.

[nickleus27]

@ice0 and @rodolforg I didn't understand ice0 example at first reading, but going through it again I see that it is a really great idea. I appreciate you showing me how to do that. I think it helped me learn. 💯 I added a commit with ice0's suggestion.

[ice0]

Hi, Nick!

I tried making it work a bool parameter, but to be honest I was a little confused how to make it work.

I made a commit to see changes in this method.
ice0@14a0253

So let's check them.

dialog->set_select_multiple(true);

Here you can use the passed parameter (bool select_multiple) to enable or disable multiple selection. Something like that:

dialog->set_select_multiple(select_multiple);

if (filename.empty())
    dialog->set_filename(prev_path);
else if (is_absolute_path(filename))
    dialog->set_filename(filename);
else
    dialog->set_filename(prev_path + ETL_DIRECTORY_SEPARATOR + filename);

This part sets the currently selected filename in the dialog.
You can check the documentation here:
https://developer-old.gnome.org/gtkmm/3.24/classGtk_1_1FileChooser.html#a35bbea274ddb0a7b948e6f23b70b9bed

This brings up an additional question. If we allow multiple selection, do we need to open a window with multiple selected files?
Or just the last selected folder?

Now this part:

for(std::string filename : filenames){
	// info("Saving preference %s = '%s' in App::dialog_open_file()", preference.c_str(), dirname(filename).c_str());
	_preferences.set_value(preference, dirname(filename));
}

Take a look at this line:

  _preferences.set_value(preference, dirname(filename));

This code saves the last opened folder in the preferences so that you can open it next time when you open the dialog box.
In your case, there is no need to iterate over the files. The directory where the files are located must always be the same.
Quote from docs:

Lists all the selected files and subfolders in the current folder of chooser.

So you can get that path from the first file.

_preferences.set_value(preference, dirname(filenames.front()));

P.S. I'm not sure if filenames can be empty after selection, so it's better to check this.

if (!filenames.empty()) {
    _preferences.set_value(preference, dirname(filenames.front()));
}

[nickleus27]

@ice0
I made a commit that fixed steps 1 and 3. However after reading the docs you provided I am still uncertain how to use this code:

if (filename.empty())
    dialog->set_filename(prev_path);
else if (is_absolute_path(filename))
    dialog->set_filename(filename);
else
    dialog->set_filename(prev_path + ETL_DIRECTORY_SEPARATOR + filename);

Originally filename was passed in from
CanvasView::import_file() as "*.*" so I am uncertain how to go about this. I could replace filename with "*.*" in these lines of code but that wouldn't work for

if (filename.empty())

would this work for this line?

if (filenames.empty())

From how the code was previously it seems that filename is always passed in as "*.*" from CanvasView::import_file(). Unless I am mis-understanding something?

[ice0]

Originally filename was passed in from CanvasView::import_file() as "." so I am uncertain how to go about this. I could replace filename with "." in these lines of code but that wouldn't work for

It is also used in Widget_Filename::on_button_choose_pressed.
In order not to break compatibility, we can use the following logic:
If the user wants to open one file, the logic is the same as now (we just put the filename as the first element in the vector)

App::dialog_open_file(const std::string &title, std::string &filename, std::string preference)
{
	std::vector<std::string> filenames;
        if (!filename.empty()) {
               filenames.push_back()
        }
	if(dialog_open_file_ext(title, filenames, preference, false)) {
		filename = filenames.front();
		return true;
	}
	return false;
}

If the user wants to open multiple files, then we we open last opened directory, without selecting any files.
In this case, we can rewrite code like this:

if (filenames.empty()) // No filename was passed
    dialog->set_filename(prev_path);
else if (is_absolute_path(filenames.front()))
    dialog->set_filename(filenames.front());
else
    dialog->set_filename(prev_path + ETL_DIRECTORY_SEPARATOR + filenames.front());

From how the code was previously it seems that filename is always passed in as "." from CanvasView::import_file(). Unless I am mis-understanding something?

Yes, but it can be called with different filename mask from Widget_Filename::on_button_choose_pressed.

[nickleus27]

@ice0 Thank you, That makes sense now. I failed to recognize Widget_Filename::on_button_choose_pressed . I should look through the code more next time before asking the question.

[nickleus27]

Hi @ice0 and @rodolforg! I think these changes are looking a lot better. Thank you for all the help along the way.

[nickleus27]

@ice0 Hey ice how are you? As you can probably tell I am a super newb at github. I tried to git merge and then git push to keep this branch up to date with master. But after doing so it added other peoples commits it seems. Did i do this correctly? Thanks for being patient with answering my questions.

[ice0]

Hi, Nick!

Please try git pull --rebase origin master command to move your changes on top of master branch.

After that you will need to force push your changes (since this rewrites git history).
git push <remote> --force

Good documentation on rebase here:
https://git-scm.com/docs/git-rebase

[nickleus27]

@ice0 thank you ice! Much appreciated!

[nickleus27]

@ice0 Thank you that worked great, and this is looking a lot better now. So git pull --rebase origin master is what I want to use to keep the pull request up to date with master? Also, thanks for the link to the documentation. There is a lot of information there. I am currently reading it.

[nickleus27]

I am excited to say I figured out what I did wrong with the previous push. :)

[ice0]

@nickleus27
Looks good, but I want to suggest one more improvement.
After importing several files, only one of them is selected in the layer list. I suggest to make all of them selected.

[nickleus27]

@ice0 can you look over my imported files selected in layers list commit to see if looks okay. Thanks.

[ice0]

@nickleus27 Looks good to me!

[ice0]

Merged. Thank you!