Fix for issue 3143: Import entry from clipboard in different formats

[125m125]

see #3143
Use all importers to parse pasted entries. This allows the user to paste entries in different formats and not ony BibTex.

• Change in CHANGELOG.md described
• Tests created for changes
• Manually tested changed features in running JabRef
• Check documentation status (Issue created for outdated help page at help.jabref.org?)

[LinusDietz]

Hi @125m125, as I see you have some troubles with our checkstyle rules. I recommend you to use our IDE config files from https://github.com/JabRef/jabref/tree/master/config. Which IDE are you using? We recommend IntelliJ IDEA or Eclipse.
You can see if the tests a pass by calling gradle check. If you only want to check for the code style rules, you can call gradle checkstyleMain.

If you have any questions, feel free to ask them at slack: https://jabref.slack.com

[125m125]

I am using Eclipse and just had to format a file. Because the saveAction is set to only reformat edited lines a missing newline in the imports wasn't corrected. Also thanks for pointing out gradle check. I somehow missed that command.

[Siedlerchr]

@125m125 If you use eclipse, just rungradlew eclipseto import the correct code formatting rules

[tobiasdiez]

Thank you for your contribution!

The code looks fine to me for the majority. Especially, I really like the new added tests. Nonetheless, I have a few remarks that I would like to see fixed before your PR is merged.

[tobiasdiez]

I'm a big fan of these tests! The only thing that I don't like is this hack to set CLIPBOARD. Instead I propose to convert

jabref/src/main/java/org/jabref/gui/ClipBoardManager.java

Line 30 in 32e934f

private static final Clipboard CLIPBOARD = Toolkit.getDefaultToolkit().getSystemClipboard();

to an instance variable which is initialized in the constructor. Then you can just use clipBoardManager = new ClipBoardManager(clipboard, importFormatReader).

[tobiasdiez]

As far as I can see, this is mostly a copy of importUnknownFormat(Path filePath) and I would like to have a solution without code duplication. Maybe something along the following lines works?

private UnknownFormatImport importUnknownFormat(Function<Importer, ParserResult> importDatabase, Function<Importer, Boolean> isRecognizedFormat) {
    // original method with obvious changes imFo.importDatabase -> importDatabase.apply(imFo)
}

public UnknownFormatImport importUnknownFormat(Path filePath) {
    return importUnknownFormat(importer -> importer.importDatabase(filePath, importFormatPreferences.getEncoding(), importer -> importer .isRecognizedFormat(filePath, importFormatPreferences.getEncoding());
}

public UnknownFormatImport importUnknownFormat(String data) {
    return importUnknownFormat(importer -> importer.importDatabase(data), importer -> importer .isRecognizedFormat(data));
}

It's a bit ugly. Do you have a better idea to reduce the code duplication?

[tobiasdiez]

Can you please extract the method Importer.importDatabase(String data). This new method can be reused at least in the MrDLibFetcher. Similarly, for the isRecognizedFormat.

[tobiasdiez]

Why is this special treatment necessary for the import from a string, but not when the same content is imported from a file?

[125m125]

Thanks for the feedback. I integrated the requests with the last commit.
I also changed the PdfXmpImporter to return false when checking the format instead of throwing an exception to prevent the need for special treatment and make it consistent with the PdfContentImporter.

[tobiasdiez]

Thanks for the quick follow-up! The code looks very good now. I give my +1 for merge (although I have to admit that I've not tested the new functionality).

I think, however, that we should wait with merging it until 4.0 is released.

[lenhard]

Ok, then I'll add this to the 4.1 milestone.

[lenhard]

The code looks fine. The tests are especially nice!

I tested this PR in a running JabRef with the example linked in #3143. The pasting works, but I always get this message in the log: [Fatal Error] :1:1: Content is not allowed in prolog. I am not 100% sure where it comes from, can you please investigate? Also, is there a way of avoiding it, after all the pasting works regardless?

[tobiasdiez]

I think the error message comes from

jabref/src/main/java/org/jabref/logic/importer/fileformat/MrDLibImporter.java

Lines 53 to 60 in ae6d01b

	try (InputStream stream = new ByteArrayInputStream(recommendationsAsString.getBytes())) {
	saxParser.parse(stream, handler);
	} catch (Exception e) {
	LOGGER.error(e.getMessage(), e);
	return false;
	}
	} catch (ParserConfigurationException | SAXException e) {
	LOGGER.error(e.getMessage(), e);

In my opinion, you can just delete these Logger.log statements (since its quite expected that a file in a wrong format is passed to isRecognizedFormat and this importer is not used outside of internal code anyways).