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

Fix pdf file import when only one file selected and no entry created #2592

Merged
merged 2 commits into from
Feb 28, 2017

Conversation

Siedlerchr
Copy link
Member

Fixes #2577
Problem is not the filename
Problem: You still get a warning that one file could not be imported, although it successfully worked.

I have unfortunately no idea if this code is at all correct, the DroppedFileHandler stuff looks suspicious to me, as it is normally used wit Drag and drop.

Question: Is there somewhere a method for linking pdfs?

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@Siedlerchr Siedlerchr added status: help wanted status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Feb 27, 2017
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Can you please add a changelog entry and then directly merge.

There is a class for linked files (something like (Parsed)FileField?). However, it is not used everywhere...

@tobiasdiez tobiasdiez removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 27, 2017
@lenhard
Copy link
Member

lenhard commented Feb 28, 2017

I agree with Tobias. I added a changelog entry and am merging directly.

I also tried it out locally according to the description listed in #2577 and can confirm that the error no longer appears. I would not care about the warning that still appears, it is sort of valid (nothing was imported).

@lenhard lenhard merged commit 0a5c8b7 into master Feb 28, 2017
@tobiasdiez tobiasdiez deleted the fiximportpdf branch February 28, 2017 13:25
Siedlerchr added a commit that referenced this pull request Mar 2, 2017
* upstream/master: (21 commits)
  Implement #1904: filter groups (#2588)
  Braces checking followup (#2598)
  Improve braces checking (#2593)
  Fix pdf file import when only one file selected and no entry created (#2592)
  Fix test
  Imports
  Localization: MainFile: French: update (#2591)
  isPdf helper method
  Add asString with UTF-8 as standard encoding
  Fix names of groups in "add/move to group" dialog
  Make automatic groups a first-class citizien (#2563)
  Rename downloadToString method to asString
  Refactoring method names
  Inline Mime type detection
  Fix imports
  Extract SSL bypassing
  Bypass SSL functionality
  Unused methods and refactoring
  Always use browser user agent
  Monitored URLDownload is currently not use anywhere
  ...

# Conflicts:
#	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants