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

Found entries will be shown when dropping a pdf with xmp meta data #2150

Merged
merged 2 commits into from
Oct 12, 2016

Conversation

boceckts
Copy link
Contributor

@boceckts boceckts commented Oct 10, 2016

When dropping a pdf with xmp meta data to the "File" field of an entry the information dialog should diplay the found entries. See koppor#5 for reference.
The general pdf import dialog which is shown when dropping a pdf somewhere to the main table also show the found entries.

xmpmetaentries

importwithxmp

  • Internal quality assurance
  • Change in CHANGELOG.md described
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef

@tschechlovdev tschechlovdev added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed stupro-ready-for-internal-review labels Oct 10, 2016
@tschechlovdev tschechlovdev changed the title [WIP] Found entries will be shown when dropping a pdf with xmp meta data Found entries will be shown when dropping a pdf with xmp meta data Oct 10, 2016
@koppor
Copy link
Member

koppor commented Oct 11, 2016

Could you also treat the dialog which appears when dropping on an entry in the entry table:

grabbed_20161011-040244

@koppor
Copy link
Member

koppor commented Oct 11, 2016

Apart from my question: LGTM 👍

@boceckts
Copy link
Contributor Author

I also added the found entries to the gerneral pdf import dialog and updated the description of this pr.

@koppor
Copy link
Member

koppor commented Oct 11, 2016

Please push and do not include 2f3c47f as this seems to be from another PR.

grabbed_20161011-182013

@boceckts
Copy link
Contributor Author

Sry I guess I forgot to push...
The commit history looks good in my opinion. Here I can only see the two commits I made and all the changes shown in this pr are from me.

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.

LGTM

@oscargus
Copy link
Contributor

Looks good!

Would it make sense to show the resulting entry independent of which option is selected in the "Import metadata from PDF" dialog? (The heading of the source box needs to be changed in that case.)

@boceckts
Copy link
Contributor Author

@oscargus what do you mean by that? The entry from the XMP metadata will always be shown if an entry is found, regardless of what is selected. The "XMP-metadata" section is only hidden if no entry has been found.

@oscargus
Copy link
Contributor

I'm thinking that it can always be shown and it shows whatever entry is generated. For a blank it may be:

@misc{,
file = {...},
}

and so on. In this way, the user knows what the resulting entry will be.

No idea how easy it is (i.e., when the actual entry is created and therefore if one can get hold of it to show) and if it should be in this PR. Just seemed like a nice extension to the general idea of this PR.

@boceckts
Copy link
Contributor Author

I get the entries from the XMPUtil.readXMP(..) which can also return an empty list if it didn't find any entry. So I guess I would have to adjust this method to get a "blank" entry. Does it really make sense to create an empty entry?

@oscargus
Copy link
Contributor

oscargus commented Oct 11, 2016 via email

@boceckts
Copy link
Contributor Author

XMP can not be selected if there is no xmp metadata. :)

On Oct 11, 2016, 21:30, at 21:30, Oscar Gustafsson notifications@github.com wrote:

Not in that case. I'm more thinking if you select one of the other
import
cases with the radio buttons (blank with link, content). Not sure how
to
handle all possible outcomes though. What happens if you select XMP and
there is no XMP data? Anything gets added?

You can leave it for now, if you want to, as I do not know how
realistic it
is to implement it.

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#2150 (comment)

@oscargus
Copy link
Contributor

OK! Then that part is solved. :-)

@koppor koppor merged commit 4999245 into JabRef:master Oct 12, 2016
@boceckts boceckts deleted the showxmpmeta branch October 12, 2016 09:04
Siedlerchr added a commit that referenced this pull request Oct 13, 2016
* upstream/master: (24 commits)
  hotfix NPE in the main table (#2158)
  Enhance side pane toggle (#1605)
  Added gray background text to authors field to assist newcomers (#2147)
  Rewrite google scholar fetcher to new infrastructure (#2101)
  Found entries will be shown when dropping a pdf with xmp meta data (#2150)
  Japanese translation (#2155)
  Add shortcuts to context menu (#2131)
  [WIP] Doi resolution ignore (#2154)
  Fix DuplicationChecker and key generator (#2151)
  just close popup on first ESC
  keep entry in sight
  Fix isSharedDatabaseAlreadyPresent check.
  don't change entry after search if editing an entry (#2129)
  Change download URL to downloads.jabref.org (#2145)
  fix switching edited textfield in the entry editor with TAB (#2138)
  Update me.champeau.gradle.jmh' version from 0.3.0 to 0.3.1
  Update install4j plugin from 6.1.2 to 6.1.3
  Remove gradle plugin com.github.youribonnaffe.gradle.format as it is deprecated and the config uses a non-existing file
  Update gradle from 3.0 to 3.1
  Fix changing the font size not working when importing preferences (#2069)
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/gui/externalfiles/DroppedFileHandler.java
#	src/main/java/net/sf/jabref/gui/importer/fetcher/GoogleScholarFetcher.java
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants