-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add more file importers to JabRef #13310
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
base: main
Are you sure you want to change the base?
Conversation
Currently having errors while compiling:
|
jablib/src/main/java/org/jabref/logic/importer/fileformat/docs/OdpImporter.java
Outdated
Show resolved
Hide resolved
jablib/src/main/java/org/jabref/logic/importer/fileformat/docs/OdsImporter.java
Outdated
Show resolved
Hide resolved
Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / Checkstyle (pull_request)" and click on it. In case of issues with the import order, double check that you activated Auto Import. You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports. Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues, commit, and push. |
jablib/src/main/java/org/jabref/logic/importer/fileformat/docs/OdtImporter.java
Outdated
Show resolved
Hide resolved
jablib/src/test/java/org/jabref/logic/importer/fileformat/docs/OdpImporterTest.java
Outdated
Show resolved
Hide resolved
jablib/src/test/java/org/jabref/logic/importer/fileformat/docs/OdsImporterFilesTest.java
Outdated
Show resolved
Hide resolved
jablib/src/test/java/org/jabref/logic/importer/fileformat/docs/OdsImporterTest.java
Outdated
Show resolved
Hide resolved
jablib/src/test/java/org/jabref/logic/importer/fileformat/docs/OdtImporterTest.java
Outdated
Show resolved
Hide resolved
exclude commons logging from the dependency |
Your pull request needs to link an issue correctly. To ease organizational workflows, please link this pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue:
Examples
|
JUnit tests of You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide. |
…away common methods
|
||
@Override | ||
protected void extractAdditionalMetadata(BibEntry entry, TikaMetadataParser metadataParser) { | ||
entry.setType(BiblatexNonStandardTypes.Image); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using setter method instead of the recommended wither pattern for BibEntry modifications. Should use withType() method to maintain immutability principles.
protected void extractAdditionalMetadata(BibEntry entry, TikaMetadataParser metadataParser) { | ||
List<String> authors = ListUtil.concat(metadataParser.getDcCreators(), metadataParser.getDcContributors()); | ||
|
||
entry.setField(StandardField.AUTHOR, TikaMetadataParser.formatBibtexAuthors(authors)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using setField instead of withField violates the immutability principle. Should use entry.withField(StandardField.AUTHOR, ...) for better consistency.
import org.apache.tika.metadata.Property; | ||
|
||
public class TikaMetadataParser { | ||
private final static Pattern imageDatePattern = Pattern.compile("(year|month|day|hour|minute|second)=(\\d+)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect modifier order. According to Java conventions, 'static' should come before 'final'. Should be 'private static final Pattern'.
/** | ||
* Concatenate two {@link List}s. Does not modify the original lists. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JavaDoc comment is trivial and simply restates what is obvious from the method signature and name. It doesn't provide any additional information or reasoning about the implementation.
@trag-bot didn't find any issues in the code! ✅✨ |
@@ -181,6 +181,33 @@ dependencies { | |||
|
|||
implementation("de.rototor.snuggletex:snuggletex-jeuclid") | |||
|
|||
// region for document importing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rework with the new scheme.
But the concrete versions at https://github.com/JabRef/jabref/blob/main/versions/build.gradle.kts
|
||
requires javafx.base; | ||
requires javafx.graphics; // because of javafx.scene.paint.Color | ||
requires afterburner.fx; | ||
requires com.tobiasdiez.easybind; | ||
|
||
// for java.awt.geom.Rectangle2D required by org.jabref.logic.pdf.TextExtractor | ||
requires java.desktop; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is org.jabref.logic.pdf.TextExtractor
gone? Then also delete the line before.
@@ -106,16 +106,14 @@ | |||
exports org.jabref.logic.git; | |||
exports org.jabref.logic.pseudonymization; | |||
exports org.jabref.logic.citation.repository; | |||
|
|||
requires java.base; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, really, no java.base
any more?
@@ -252,5 +250,7 @@ | |||
requires mslinks; | |||
requires org.antlr.antlr4.runtime; | |||
requires org.libreoffice.uno; | |||
requires org.apache.tika.core; | |||
requires org.jetbrains.annotations; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please no jetbrains annotations
Use JSpecify - "howto" at https://www.jetbrains.com/help/idea/annotating-source-code.html#configure-nullability-annotations
@@ -49,6 +49,10 @@ public abstract class Importer implements Comparable<Importer> { | |||
* @throws IOException Signals that an I/O exception has occurred. | |||
*/ | |||
public boolean isRecognizedFormat(Path filePath) throws IOException { | |||
if (!Files.exists(filePath) || !Files.isRegularFile(filePath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latter includes the former, doesn't it?
if (!Files.exists(filePath) || !Files.isRegularFile(filePath)) { | |
if (!Files.isRegularFile(filePath)) { |
import org.jabref.logic.util.FileType; | ||
import org.jabref.logic.util.StandardFileType; | ||
|
||
public class DjvuImporter extends TikaImporter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some refernce to the format would be nice.
import org.jabref.logic.util.FileType; | ||
import org.jabref.logic.util.StandardFileType; | ||
|
||
public class EpubImporter extends TikaImporter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some refernce to the format would be nice.
(Even if it self-explanatory here; but mayb e some more non-obvious onformatoin can be found there)
Changes made:
Steps to test
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if change is visible to the user)