Skip to content

Fix for issue 6199: EndNote .xml import to JabRef: PDF links are not imported corrected.#7667

Merged
Siedlerchr merged 4 commits into
JabRef:mainfrom
devinluo27:aloofwisdom-branch
Apr 26, 2021
Merged

Fix for issue 6199: EndNote .xml import to JabRef: PDF links are not imported corrected.#7667
Siedlerchr merged 4 commits into
JabRef:mainfrom
devinluo27:aloofwisdom-branch

Conversation

@devinluo27

Copy link
Copy Markdown
Contributor

Fixes #6199

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Summary of Issue 6199

When users import EndNote type xml to Jabref, it always assumes that urls are warped inside a style tag. However, some urls may not be wrapped in a style tag and as a result, Jabref cannot parses them.

Way to Fix

In the xsd file, I change the the complexType's attribute to mix="true", so that it can accept both vanilla text and subelements. Then I will check whether a style tag exists: if it does, use the text inside it; otherwise, use vanilla text instead.

@Siedlerchr Siedlerchr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a minor code improvement, but otherwise looks good to me!
That will benefit a lot of users!

Comment thread src/main/java/org/jabref/logic/importer/fileformat/EndnoteXmlImporter.java Outdated
</xs:element>
<xs:element name="url">
<xs:complexType>
<xs:complexType mixed="true">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah cool, did not know about this!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yah, it's funny. I just knew that when I was looking up some relevant docs yesterday.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does anyone know whether there is a globally updated "endnote.xsd"?

I am of the community believing that schema schecking is a good thing (json schema, xml schema, ...). (Side note: some say that even the schema should be sent along with the message: https://microservice-api-patterns.org/patterns/structure/elementStereotypes/MetadataElement#)

If a third party releases an schema for their data meaning that data produced by their software can be read in case it conforms to that schema, I would not try to change that schema.

Google lead me to https://forums.zotero.org/discussion/26493/endnote-xml-export-import - however, this his very outdated. Maybe, endnote.xsd is not maintained anymore and we can go ahead to create a fork?

I just wanted to double check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you mean a globally standardized version of "endnote.xsd"?

I do not know much about it yet. But sending messages with its corresponding schema is indeed reasonable in order to parse the info correctedly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Endnote only has a dtd not an xsd https://support.clarivate.com/Endnote/s/article/EndNote-XML-Document-Type-Definition?language=en_US
Seems like the xsd was somehow generated from an xml file. The xsd has been in JabRef since ages

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 24, 2021
@devinluo27

Copy link
Copy Markdown
Contributor Author

Just a minor code improvement, but otherwise looks good to me!
That will benefit a lot of users!

I kind of messed up the previous commits. So I have incorporated your suggestion in the latest commit b1c3f75, please check.

@Siedlerchr

Copy link
Copy Markdown
Member

@Aloofwisdom please have a look at the checkstyle test. You can also see the comments from reviewdog when you look at the Files changed tab

@devinluo27

Copy link
Copy Markdown
Contributor Author

@Aloofwisdom please have a look at the checkstyle test. You can also see the comments from reviewdog when you look at the Files changed tab

I could pass the CheckStyle locally now. Please check again.

@Siedlerchr Siedlerchr merged commit 21abebe into JabRef:main Apr 26, 2021
@Siedlerchr

Copy link
Copy Markdown
Member

Thanks a lot for your contribution!

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.

EndNote .xml import to JabRef: PDF links are not imported correctly.

3 participants