-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix package of PreviewLayout #5702
Conversation
Would it be possible to convert the antlr3 grammar to antlr4? |
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.
I have to admit I also don't really see the point in supporting bst. Especially, if it is only one built-in preview. In this case I would have preferred that you convert the bst to a cls citation style.
Use case: A user uses bibtex to render his bibliography. This still happens out there. E.g., IEEE and Springer. An observed major WTF of JabRef was that the entry preview for a concrete paper was different than the rendering by LaTeX. With this PR, it is optimized for the use case to use bibtex in a concrete paper. It is going to play well with the latex integration. One can see the citations in JabRef as they are rendered in the current (!) paper. The other possibility is to add a map of existing We can also offer the BST functionality as library. Refs #110. |
b8ef7b7
to
21c6e5e
Compare
# Conflicts: # src/main/java/org/jabref/gui/maintable/RightClickMenu.java # src/main/java/org/jabref/gui/preview/CopyCitationAction.java # src/main/java/org/jabref/gui/util/CustomLocalDragboard.java # src/main/java/org/jabref/preferences/JabRefPreferences.java
@Siedlerchr It is very difficult to convert the ANTLRv3 grammar to ANTLRv4. The grammar itself is very simple. I still could not manage to update it. So, I would aks to keep it as is. |
Since the PR also contrains refactorings, I removed the UI code and left the logic in. That could be picked up in a future point in time. |
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.
Generally looks fine to me.
Please don't forget to add a changelog entry.
@@ -1,8 +1,11 @@ | |||
package org.jabref.logic.citationstyle; | |||
package org.jabref.logic; |
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 did you moved this? I find citationstyle
is a pretty fitting package for all preview-related stuff. PreviewLayout
is definitely not important enough to directly lie in the logic
package.
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.
@@ -0,0 +1,88 @@ | |||
package org.jabref.logic.bst; |
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.
I would move this also to the citationstyle
package, because it's not really about bst
(which is only used as a tool)
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.
I see org.jabref.logic.preview
as the other possibility to collect the preview generators. -- I would keep the package org.jabref.logic.citationstyle
for CSL only and keep out the relation to .bst
and to our other Layouts.
public void generatePreviewForSliceTheoremPaperUsingAbbr() throws Exception { | ||
BstPreviewLayout bstPreviewLayout = new BstPreviewLayout(Paths.get(BstPreviewLayoutTest.class.getResource("abbrv.bst").toURI())); | ||
String preview = bstPreviewLayout.generatePreview(getSliceTheoremPaper(), bibDatabase); | ||
assertEquals("T. Diez. Slice theorem for fréchet group actions and covariant symplectic field theory. #may# 2014.", preview); |
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.
Shouldn't #may#
be displayed properly?
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.
Had to dive deeply in our writing logic to get this fixed. Added some JavaDoc comments and (slightly) improved the code of our writers.
I understood that I need "FieldWriter" 😇 Did some quick hacks to use the result of it. I did not want to touch FieldWriter only for this functionality.
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.
I'm not a big fan of having a preview
package in logic
. The logic package shouldn't care about where the functionality is used. I'm in favor of having one package where all BibEntry to text representation code is gathered (old layout
files, CSL and bst). But then again that's also not soooo important...
@@ -1174,25 +1174,17 @@ private void strings(Tree child) { | |||
|
|||
public static class BstEntry { | |||
|
|||
private final BibEntry entry; | |||
public final BibEntry entry; |
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.
Then this triggers a follow-up question (@LinusDietz): is it ok to make variables with mutable types (like Map<>
) public via public final
? Because then outside code can modify the content of a class to their liking (which I guess is against some of the principles of object-oriented programming).
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.
This is effective Java #39. "Make defensive copies when needed".
You must program defensively, with the assumption that clients of your class will do their best to destroy its invariants.
However, did not see that being done much, so I am curios to the opinion of @LinusDietz
@tobiasdiez I put it in separate packages, because I favor "cohesion" (?) of packages. The cohesion in the three separate packages is high and would get lower if I would merge them. The coupling of packages does not change in both cases (does it?). I only found that page on the concept http://prinzipien-der-softwaretechnik.blogspot.com/2013/02/kopplung-und-kohasion-fundamentale-oder.html... - Should we discuss personally? (Maybe I misunderstood you - Regarding |
Think, the code is good to go. We need to do the architecture things afterwards. |
0654e16 Create scandinavian-journal-of-information-systems.csl (#5716) ce2d537 Update journal-of-computer-applications-in-archaeology.csl (#5715) 755d3d3 Create human-rights-law-review.csl (#5626) 0feda94 Create journal-of-intercultural-studies.csl (#5709) ae4756d Update acta-universitatis-agriculturae-sueciae.csl (#5713) 323d9ac Update mohr-siebeck-recht.csl (#5559) 15530a8 Bch corr (#5712) 094a1af Create forschungsjournal-soziale-bewegungen-fjsb.csl (#5699) cb91566 initialize authors and editors (#5714) 2d5cfff Create cancer-biomarkers.csl (#5703) 5e264d5 Update multidisciplinary-digital-publishing-institute.csl (#5708) 46e961f Create klinische-padiatrie.csl (#5711) e81e877 Create bulletin-archeologique-des-ecoles-francaises-a-l-etranger.csl (#5704) 0029c5a Create polar-research.csl 🧊 (#5702) 7db1361 Update vancouver-imperial-college-london.csl (#5641) b953e9f Update iso690-author-date-fr-no-abstract.csl (#5706) 91eda8c Update thieme-german.csl (#5710) ebe0787 Update harvard-imperial-college-london.csl (#5643) 2d4db76 Fix UNESCO IIEP in text 436cbf4 Create revue-archeologique-de-narbonnaise.csl (#5688) 5150bcf Create journal-of-computer-assisted-tomography.csl (#5690) dd6f050 Create anti-trafficking-review.csl (#5658) 08e622f Create the-angle-orthodontist.csl (#5685) c6a1907 journal-of-palm-oil-research.csl fix several errors (#5686) 6cbe29d Create bern-university-of-applied-sciences-school-of-agricultural-for… (#5684) f590dc1 Update biomed-central.csl (#5701) 1efce81 Update turabian-author-date.csl (#5695) 12dbba5 Create tyndale-bulletin (#5673) b0746db Create Engineered Regeneration (#5682) e38b953 wikipedia citation template (#5662) 5e7f731 Create early-music-history.csl (#5679) 86443f3 Create zeitschrift-fur-politik.csl (#5676) 68f1996 Create annals-of-work-exposures-and-health.csl (#5666) 1ba9dc6 Create brazilian-journal-of-psychiatry.csl (#5672) 438f92c fix error for speech in ama styles (#5693) 7a0c2d3 set initialize-with-hyphen to false (#5689) 3bd2765 Update emu-austral-ornithology.csl (#5671) 31492b2 fix various errors in natura-croatica.csl (#5687) 94d6b23 Update iso690-author-date-cs.csl (#5677) 5d017da minor update on the "Haute école de gestion de Genève - ISO 690" style (#5665) 2cad8f6 add ibid/subsequent to comparative-politics.csl (#5669) de0b116 Create taylor-and-francis-vancouver-national-library-of-medicine.csl (#5650) ed87f99 Update bulletin-de-correspondance-hellenique.csl (#5663) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 0654e16
* Squashed 'buildres/csl/csl-styles/' changes from 3a6a0a7..0654e16 0654e16 Create scandinavian-journal-of-information-systems.csl (#5716) ce2d537 Update journal-of-computer-applications-in-archaeology.csl (#5715) 755d3d3 Create human-rights-law-review.csl (#5626) 0feda94 Create journal-of-intercultural-studies.csl (#5709) ae4756d Update acta-universitatis-agriculturae-sueciae.csl (#5713) 323d9ac Update mohr-siebeck-recht.csl (#5559) 15530a8 Bch corr (#5712) 094a1af Create forschungsjournal-soziale-bewegungen-fjsb.csl (#5699) cb91566 initialize authors and editors (#5714) 2d5cfff Create cancer-biomarkers.csl (#5703) 5e264d5 Update multidisciplinary-digital-publishing-institute.csl (#5708) 46e961f Create klinische-padiatrie.csl (#5711) e81e877 Create bulletin-archeologique-des-ecoles-francaises-a-l-etranger.csl (#5704) 0029c5a Create polar-research.csl 🧊 (#5702) 7db1361 Update vancouver-imperial-college-london.csl (#5641) b953e9f Update iso690-author-date-fr-no-abstract.csl (#5706) 91eda8c Update thieme-german.csl (#5710) ebe0787 Update harvard-imperial-college-london.csl (#5643) 2d4db76 Fix UNESCO IIEP in text 436cbf4 Create revue-archeologique-de-narbonnaise.csl (#5688) 5150bcf Create journal-of-computer-assisted-tomography.csl (#5690) dd6f050 Create anti-trafficking-review.csl (#5658) 08e622f Create the-angle-orthodontist.csl (#5685) c6a1907 journal-of-palm-oil-research.csl fix several errors (#5686) 6cbe29d Create bern-university-of-applied-sciences-school-of-agricultural-for… (#5684) f590dc1 Update biomed-central.csl (#5701) 1efce81 Update turabian-author-date.csl (#5695) 12dbba5 Create tyndale-bulletin (#5673) b0746db Create Engineered Regeneration (#5682) e38b953 wikipedia citation template (#5662) 5e7f731 Create early-music-history.csl (#5679) 86443f3 Create zeitschrift-fur-politik.csl (#5676) 68f1996 Create annals-of-work-exposures-and-health.csl (#5666) 1ba9dc6 Create brazilian-journal-of-psychiatry.csl (#5672) 438f92c fix error for speech in ama styles (#5693) 7a0c2d3 set initialize-with-hyphen to false (#5689) 3bd2765 Update emu-austral-ornithology.csl (#5671) 31492b2 fix various errors in natura-croatica.csl (#5687) 94d6b23 Update iso690-author-date-cs.csl (#5677) 5d017da minor update on the "Haute école de gestion de Genève - ISO 690" style (#5665) 2cad8f6 add ibid/subsequent to comparative-politics.csl (#5669) de0b116 Create taylor-and-francis-vancouver-national-library-of-medicine.csl (#5650) ed87f99 Update bulletin-de-correspondance-hellenique.csl (#5663) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 0654e16 * Squashed 'buildres/csl/csl-locales/' changes from 0cc3885f61..d5ee85de8e d5ee85de8e Period after Übers. added (#241) git-subtree-dir: buildres/csl/csl-locales git-subtree-split: d5ee85de8e74d4109509014758b6f496a968ff03 * fix merge error Co-authored-by: github actions <[email protected]> Co-authored-by: Siedlerchr <[email protected]>
3bb4b5f infoclio.ch styles for German: remove non-breaking space delimiters (#5754) adf28db Create journal-of-health-care-for-the-poor-and-underserved.csl (#5752) 0713a8e Update chinese-gb7714-2005-numeric.csl (#5737) 1cd3754 Update china-national-standard-gb-t-7714-2015-author-date.csl (#5746) c2536b7 Update china-national-standard-gb-t-7714-2015-numeric.csl (#5745) f8c1392 Create steel-research-international.csl (#5720) 21fe1f5 Create asian-myrmecology.csl (#5718) 91e9e2b Update harvard-university-of-the-west-of-england.csl (#5734) dd453d1 fix minor erros in polar-research.csl (#5730) 038a8f5 Remove group around no-date cluster (#5731) 0710b51 remove et-al from bibtex.csl (#5728) bbd703d Add editorial-director to universite-laval-departement-des-sciences-historiques.csl (#5727) 58ea430 Create german-journal-of-agricultural-economics.csl (#5717) 0654e16 Create scandinavian-journal-of-information-systems.csl (#5716) ce2d537 Update journal-of-computer-applications-in-archaeology.csl (#5715) 755d3d3 Create human-rights-law-review.csl (#5626) 0feda94 Create journal-of-intercultural-studies.csl (#5709) ae4756d Update acta-universitatis-agriculturae-sueciae.csl (#5713) 323d9ac Update mohr-siebeck-recht.csl (#5559) 15530a8 Bch corr (#5712) 094a1af Create forschungsjournal-soziale-bewegungen-fjsb.csl (#5699) cb91566 initialize authors and editors (#5714) 2d5cfff Create cancer-biomarkers.csl (#5703) 5e264d5 Update multidisciplinary-digital-publishing-institute.csl (#5708) 46e961f Create klinische-padiatrie.csl (#5711) e81e877 Create bulletin-archeologique-des-ecoles-francaises-a-l-etranger.csl (#5704) 0029c5a Create polar-research.csl 🧊 (#5702) 7db1361 Update vancouver-imperial-college-london.csl (#5641) b953e9f Update iso690-author-date-fr-no-abstract.csl (#5706) 91eda8c Update thieme-german.csl (#5710) ebe0787 Update harvard-imperial-college-london.csl (#5643) 2d4db76 Fix UNESCO IIEP in text 436cbf4 Create revue-archeologique-de-narbonnaise.csl (#5688) 5150bcf Create journal-of-computer-assisted-tomography.csl (#5690) dd6f050 Create anti-trafficking-review.csl (#5658) 08e622f Create the-angle-orthodontist.csl (#5685) c6a1907 journal-of-palm-oil-research.csl fix several errors (#5686) 6cbe29d Create bern-university-of-applied-sciences-school-of-agricultural-for… (#5684) f590dc1 Update biomed-central.csl (#5701) 1efce81 Update turabian-author-date.csl (#5695) 12dbba5 Create tyndale-bulletin (#5673) b0746db Create Engineered Regeneration (#5682) e38b953 wikipedia citation template (#5662) 5e7f731 Create early-music-history.csl (#5679) 86443f3 Create zeitschrift-fur-politik.csl (#5676) 68f1996 Create annals-of-work-exposures-and-health.csl (#5666) 1ba9dc6 Create brazilian-journal-of-psychiatry.csl (#5672) 438f92c fix error for speech in ama styles (#5693) 7a0c2d3 set initialize-with-hyphen to false (#5689) 3bd2765 Update emu-austral-ornithology.csl (#5671) 31492b2 fix various errors in natura-croatica.csl (#5687) 94d6b23 Update iso690-author-date-cs.csl (#5677) 5d017da minor update on the "Haute école de gestion de Genève - ISO 690" style (#5665) 2cad8f6 add ibid/subsequent to comparative-politics.csl (#5669) de0b116 Create taylor-and-francis-vancouver-national-library-of-medicine.csl (#5650) ed87f99 Update bulletin-de-correspondance-hellenique.csl (#5663) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 3bb4b5f
This PR fixes the package of
PreviewLayout
since that class is used for both CitationStyle-based previewes as well as for self-defined layouts (see https://docs.jabref.org/setup/preview for details).The PR prepares offering entry previews based on
.bst
files. These files are "bibliography style files" (offered by BibTeX) defining the rendering of bibliopgraphic entries. See https://tex.stackexchange.com/a/85433/9075 for details. That was not too hard since JabRef contains code to interpret.bst
files. This PR enables a.bst
file to be used at the entry preview.Development on that feature has been suspended as the development team focuses on other issues: https://github.com/JabRef/jabref/projects/5
Future work
IEEEtran.bst
as the default offered preview.bst
file (maybe work for a future PR)