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

Support Annotations Created by Foxit #2878

Merged
merged 13 commits into from
Jun 1, 2017
Merged

Support Annotations Created by Foxit #2878

merged 13 commits into from
Jun 1, 2017

Conversation

LinusDietz
Copy link
Member

Fixes some problems when Annotations were created using the Foxit Reader. (cf. #2839 (comment))

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

@LinusDietz LinusDietz added external files status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jun 1, 2017
@LinusDietz
Copy link
Member Author

Based on the comments of @kafran, I have made some more changes. I think this PR is ready now.

@@ -71,6 +67,10 @@
}

private boolean isSupportedAnnotationType(PDAnnotation annotation) {
if (annotation.getSubtype().equals("Link") || annotation.getSubtype().equals("Widget")) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know how likely it is, but just to be sure you should change it to "Link".equals... otherwise there might be a potenial NPE

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, should not be the case, but that pdfbox library is really not to be trusted....

*/
public static boolean isMarkedFileAnnotationType(String annotationType) {
for (FileAnnotationType type : Collections.unmodifiableList(Arrays.stream(FileAnnotationType.values())
.filter(FileAnnotationType::isLinkedAnnotationType).collect(toList()))) {
Copy link
Member

Choose a reason for hiding this comment

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

An alternative and simpler solution would be to use Enum.valueOf... to parse the String. If it throws an Illegal Argument exception you simply return false...
Otherwise I would omit the loop and directly add a second filter Argument to the strea, and use anyMatch to return a boolean

Copy link
Member

Choose a reason for hiding this comment

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

And I don't understand why you wrap the Array in a list again

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea. fixed in b9ddbc4

Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

See the isMarkedFileannotaiton

@Siedlerchr Siedlerchr merged commit f5386b2 into master Jun 1, 2017
@Siedlerchr Siedlerchr deleted the foxit-annotations branch June 1, 2017 18:28
Siedlerchr added a commit that referenced this pull request Jun 3, 2017
* upstream/master: (38 commits)
  Add link to "feature branch workflow"
  Support Annotations Created by Foxit (#2878)
  Fixes jacoco by excluding the fetcher tests from analysis (#2877)
  Fix entry editor (#2875)
  update bcprov-jdk15on from 1.56 -> 1.57
  update assertj-swing-junit from 3.5.0 -> 3.6.0
  update mockito-core from 2.7.22 -> 2.8.9
  update jfx from 0.11.0 -> 0.11.1
  update google guava from 21.0 -> 22.0
  Fix Divide by zero exception if rows is zero in Entry Editor Tab (#2873)
  Implement #2785: resort groups using drag & drop (#2864)
  Add Library of Congress as ID-fetcher (#2865)
  Fix export and import of MS office day/year/month acessed fields (#2862)
  Adsurl to url (#2861)
  Update LICENSE.md
  Update
  Update LICENSE.md
  Update license file so that github recognize it properly
  Improve Issue Template Using a Collapsible Log Area
  Fix #2852: Improve performance of group filtering.
  ...
Siedlerchr added a commit that referenced this pull request Jun 6, 2017
* upstream/master:
  Fix DOI resolving by using https (#2889)
  Adjust tests: Ads fetcher returns url, DBLP no longer works with negative operators (#2891)
  Fix loading of preferencesService (#2882)
  Add link to "feature branch workflow"
  Support Annotations Created by Foxit (#2878)
  Fixes jacoco by excluding the fetcher tests from analysis (#2877)

# Conflicts:
#	src/main/java/org/jabref/gui/DefaultInjector.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external files 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.

3 participants