-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Rewrite Bibtexml exporter #2017
Conversation
ef200d1
to
b9980ac
Compare
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.
Looks good, only minor comments from my side.
import org.apache.commons.logging.Log; | ||
import org.apache.commons.logging.LogFactory; | ||
|
||
public class BibTeXMLExportFormat extends ExportFormat { |
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 add a javadoc comment.
Marshaller marshaller = context.createMarshaller(); | ||
marshaller.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, Boolean.TRUE); | ||
|
||
marshaller.marshal(file, new java.io.File(resultFile)); |
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.
Wrong import or why do you need java.io.File
here?
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.
not wrong, but there is also a class File that is generated by JAXB. Thats is why this is needed.
} | ||
|
||
/** | ||
* Contains same logic as the parse method, but inbook needs a special treatment, because |
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.
If it contains the same logic, please use a {@link}
annotation.
editor = {Maxima Musterfrau}, | ||
volume = {1}, | ||
number = {1}, | ||
chapter = {3}, |
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 this intentionally not being exported or is it missing in the exported xml file?
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.
Yeah, this is intentionally, because the schema doesn't have something like chapter in the InCollection type.
<file xmlns="http://bibtexml.sf.net/"> | ||
<entry id="Mustermann2016"> | ||
<inbook> | ||
<bibtexkey>Mustermann2016</bibtexkey> |
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.
Why is it beeing exported to a new node here but not in the previous test files?
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.
Only inbook has the field key, that can be set. You can look at the generated Inbook class, there's a field key. Also in the other test for Inbook there's a bibtexkey as well.
Regarding the parameterized tests, something seems to be wrong with the path of the resource files because when I run the test class in IntelliJ it doesn't show any executed tests. For me the following classes are affected:
I think it is best to move this issue to a new pr. |
@@ -141,7 +140,7 @@ private void createMarshallerAndWriteToFile(File file, String resultFile) throws | |||
} | |||
|
|||
/** | |||
* Contains same logic as the parse method, but inbook needs a special treatment, because | |||
* Contains same logic as the {@link parse()} method, but inbook needs a special treatment, because |
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.
To link to a method you need to use the following annotation {@link #parse()}
, But the rest is good to me.
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.
Good! I merge it in!
* rewrite exporter with jaxb parser * add test * fix codacy and some code refactorings * delete old layout files * adress comments
Since the importer is written with a JAXB parser, I've rewritten the exporter with a jaxb parser as well. The exporter is now written in a similar way to the importer.
This should also fix #1337.