Skip to content
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

Download many files in one go #1869

Merged
merged 4 commits into from
Aug 27, 2016
Merged

Download many files in one go #1869

merged 4 commits into from
Aug 27, 2016

Conversation

oscargus
Copy link
Contributor

Implements #1496 and also allows selecting multiple documents to find full text documents for. I think that it doesn't always show a dialog for every failed files when having multiple ongoing downloads.

Please try it out at http://builds.jabref.org/downloadmanyfiles

  • Change in CHANGELOG.md described
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@Siedlerchr
Copy link
Member

If it does what you promise, I love you! 😍 😆 Will try this tomorrow.

think that it doesn't always show a dialog for every failed files when having multiple ongoing downloads.

Not sure if I understand you correctly: So I download e.g. 20 files, and 5 fails. Is then the dialog "Failed DL" displayed 5 times?
Or after all downloads are finished it shows a dialog with e.g. 15/20 files downloaded succesfully?

Localization.lang("Directory not found"), JOptionPane.ERROR_MESSAGE);
return;
}
// TODO: this needs its own thread as it blocks the UI!
Copy link
Member

Choose a reason for hiding this comment

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

Is this blocking still a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it was before either...

@Siedlerchr
Copy link
Member

Siedlerchr commented Aug 27, 2016

Works technically fine, but I have some remarks, as it's currently not effective usable:
I ran it over Quality->Look up fulltext
When the process is finished, it shows the file save/ Download failed dialog for every entry which I previously selected.
So if I have 200 entries select, after the process it opens 200 dialogs... Even for canceling I would have to click 200 times of cancel.
And There is apparently no option to cancel the lookup process.

I would suggest having just a final download result dialog or directly a process window, perhaps sth like this from Down Them All:
dta

@oscargus
Copy link
Contributor Author

Well, what you select is what you get. :-)

I agree that your approach would be even better, but that is out of the scope for me at the moment. One could still reenable one selected entry at a time, with the added benefit of being able to start more than one, but I do not really know if that is a better way to do it. Maybe it is better to pop up a dialog asking if the user is sure they want to try to find > 10(?) full text documents as this can be quite a hassle?

@Siedlerchr
Copy link
Member

Agree with you. Add this dialog warning, for more than x? entries .
For the rest we should create a new issue/enhancement. Mabye the other devs have some other

Regarding the blocking: Does not block the UI atm.

@oscargus
Copy link
Contributor Author

I updated and set it to 10. Not sure how much one should actually write in the dialog...

@@ -44,6 +44,20 @@ public void init() throws Throwable {

@Override
public void run() {
if (basePanel.getSelectedEntries().size() >= 10) {
Copy link
Member

Choose a reason for hiding this comment

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

Please extract that "magic number" to a constant.
https://en.wikipedia.org/wiki/Magic_number_(programming)

@Siedlerchr
Copy link
Member

Code LGTM, just move that magic number to a constant.

@oscargus
Copy link
Contributor Author

Yeah, was thinking about the constant thing as well. Will fix it. I guess it is a good idea to merge now so people have time to find possible issues.

@oscargus
Copy link
Contributor Author

Improved the file name generation as well. If the formatter does return an empty string, the BibTeX key is used. If no BibTeX key, "default" is used.

@oscargus oscargus merged commit b8d8f89 into master Aug 27, 2016
@oscargus oscargus deleted the downloadmanyfiles branch August 27, 2016 13:52
ayanai1 pushed a commit to ayanai1/jabref that referenced this pull request Sep 5, 2016
* Download many files in one go

* Added warnings dialog if 10 or more entries are selected for full text download

* Extracted magic number and fixed file name generation fallback

* Some code cleanups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants