Support CitationStyles #1928

chriba · 2016-09-06T16:06:46Z

Implements #119.

The User can choose multiple CitationStyles which he can cycle threw in the preview panel (forward with F9, backwards shift + F9, both are in the context menu and the menu bar) .
As discussed I removed the 2nd Preview.

Preview before:

Preview after:

Preferences before:

Preferences after:

I didn't include the localization (other the English) on purpose because they create unnecessary conflicts when rebasing quite often (I push them when everything else is fine).

The Help page has to be updated too. I do that as soon as this PR is merged.

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

Warning: currently I read the available CitationStyles at runtime. I had problems reading them from the gradle dependency thus currently they are only found when JabRef is started from the JAR.

boceckts · 2016-09-10T14:52:43Z

The text of the preferences tab is not completely visible, either reword or adjust the width of the side panel.

boceckts · 2016-09-10T15:18:26Z

src/main/java/net/sf/jabref/gui/JabRefFrame.java

@@ -638,8 +642,12 @@ public void windowClosing(WindowEvent e) {
                return;
            }

+            if (bp.getPreviewPanel() != null) {
+                bp.getPreviewPanel().updateLayout();


Please rename

boceckts · 2016-09-10T15:34:00Z

When pressing the up or down button the selected entry becomes deselected and I have to select it again, this can be quite frustrating if I want to move a citation style to the top. Can you please change it so the selected entry stays selected?

boceckts · 2016-09-10T15:39:58Z

src/main/java/net/sf/jabref/gui/preftabs/PreviewPrefsTab.java

+            if (CitationStyle.isCitationStyleFile(style)){
+                chosenModel.addElement(CitationStyle.createCitationStyleFromFile(style));
+            }
+            else {


Weird formatting, move this to the end of the line above.

boceckts · 2016-09-10T16:00:32Z

The preview panel is not being updated after changing the citation style but rather displays always the first selected entry.

Steps to reproduce:

go to preferences and add a citation style
In the main table select an entry and open the preview panel
press F9 to see the newly added citation style
select a different entry and the preview panel will not update but use the information of the first selected entry EDIT: it will update the citation style though

boceckts · 2016-09-10T16:09:41Z

The preview panel is not being updated after changing the citation style when working with multiple databases.

Steps to reproduce:

go to preferences and add a citation style
open at least two databases with at least one entry
in the main table select an entry and open the preview panel
select the second database and an entry
press F9 to change to the newly added citation style
the preview panel will not update at all but the status says the citation style has changed
When switching to the first database the preview panel has updated and responds to citation style changes.

boceckts · 2016-09-10T16:31:05Z

I think it would be great to also add the selected citation styles to the "Export to clipboard" dialog.

boceckts · 2016-09-10T16:40:36Z

src/main/java/net/sf/jabref/gui/worker/CitationStyleWorker.java

+import org.apache.commons.logging.LogFactory;
+
+
+public class CitationStyleWorker extends SwingWorker<String, Void> {


Please add a javadoc comment.

boceckts · 2016-09-10T16:59:06Z

When the bugs are fixed and the other minor comments have been addressed then it's fine by me.

koppor · 2016-09-12T08:14:48Z

build.gradle

@@ -118,6 +121,12 @@ dependencies {
    compile 'org.apache.logging.log4j:log4j-api:2.6.2'
    compile 'org.apache.logging.log4j:log4j-core:2.6.2'

+    // need to use snapshots as the stable version is from 2013 and doesn't support v1.0.1 CitationStyles
+    compile 'org.citationstyles:styles:1.0.1-SNAPSHOT'


Please also follow https://github.com/JabRef/jabref/blob/master/CONTRIBUTING.md#when-adding-a-library

Is there an issue asking for a release of the library? - If yes, please link it here and open an issue at jabref/issues linking that issue. If no, please either contact the maintainers directly or open an issue at their repo. Also open an issue at jabref/issues.

chriba · 2016-09-12T14:22:17Z

Fixed the Bug (both were caused by the same thing)
Is the Export to clipboard important? You can already copy the current preview via context menu on the preview panel.

koppor · 2016-09-12T21:30:16Z

~~The context menu exports text only. Is it possible to export the HTML?~~ When pasting into Microsoft Word, everything is formatted correctly.

The "export to clipboard" is not important. If some interested user requests it, work on it can be done.

koppor · 2016-09-12T21:32:02Z

CHANGELOG.md

@@ -12,6 +12,8 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 ## [Unreleased]

 ### Changed
+- Added support for over 1000 (self updating) 1.0.1 Citation Styles
+- You can set and cycle different preview Styles (including CitationStyles)


"styles" with lower case s

tobiasdiez · 2016-09-14T17:16:22Z

src/main/java/net/sf/jabref/logic/citationstyle/CitationStyleGenerator.java

+/**
+ * WARNING: The generation of a citation may take some time, better call it in outside the main Thread
+ */
+public class CitationStyleGenerator {


You can also mark the class as protected so that nobody outside of the package can use it (i.e. everybody is forced to use the cache).

why not package-private?

I don't like package-private (because writing nothing looks like somebody just forgot to specify the correct visibility)

tobiasdiez

The code looks really good. Just some minor remarks (mostly around Optionals) and please add a few tests.

tobiasdiez · 2016-09-15T21:06:16Z

CHANGELOG.md

@@ -20,6 +20,8 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - The search result Window won't stay on top anymore if the main Window is focused and will be present in the taskbar
 - The user can jump from the searchbar to the maintable  with `ctrl+enter`
 - Implemented [#573 (comment)](https://github.com/JabRef/jabref/issues/573#issuecomment-232284156): Added shortcut: closing the search result window with `ctrl+w`
+- Added support for over 1000 1.0.1 CitationStyles


Maybe reformulate to 1000 citation styles (using CitationStyles 1.0.1) and add link.

tobiasdiez · 2016-09-15T21:13:42Z

src/main/java/net/sf/jabref/gui/PreviewPanel.java

+        String style = previewPreferences.getPreviewCycle().get(previewPreferences.getCyclePreviewPosition());
+
+        if (CitationStyle.isCitationStyleFile(style)) {
+            if (getBasePanel() != null) {


basepanel.ifPresent(...) and maybe remove getBasePanel

tobiasdiez · 2016-09-15T21:14:21Z

src/main/java/net/sf/jabref/gui/PreviewPanel.java

+            }
+        } else {
+            updatePreviewLayout(previewPreferences.getPreviewStyle());
+            if (getBasePanel() != null) {


Same as above

tobiasdiez · 2016-09-15T21:15:23Z

src/main/java/net/sf/jabref/gui/PreviewPanel.java


+        if (citationStyleWorker != null){


Make citationStyleWorker also an Optional.

tobiasdiez · 2016-09-15T21:17:27Z

src/main/java/net/sf/jabref/gui/maintable/MainTableSelectionListener.java

-                .addSearchListener(previewPanel[1]);
-
-        this.preview = previewPanel[activePreview];
+        if (panel.getPreviewPanel() != null){


Make getPreviewPanel return an Optional

tobiasdiez · 2016-09-15T21:27:28Z

src/main/java/net/sf/jabref/logic/citationstyle/CitationStyleCache.java

+    }
+
+    public void setCitationStyle(CitationStyle citationStyle) {
+        if (citationStyle == null || this.citationStyle == citationStyle) {


I would guard citationStyle from being null (Objects.requireNonNull).

tobiasdiez · 2016-09-15T21:28:12Z

src/main/java/net/sf/jabref/logic/citationstyle/CitationStyleCache.java

+        if (citationStyle == null || this.citationStyle == citationStyle) {
+            return;
+        }
+        if (this.citationStyle == null || !this.citationStyle.getSource().equals(citationStyle.getSource())) {


Overwrite equals in CitationStyle and reuse here.

tobiasdiez · 2016-09-15T21:29:09Z

src/main/java/net/sf/jabref/logic/citationstyle/CitationStyleCache.java

+         */
+        @Subscribe
+        public void listen(EntryChangedEvent entryChangedEvent) {
+            if (entryChangedEvent != null && entryChangedEvent.getBibEntry() != null){


Can the event and/or the BibEntry ever be null?

The event itself shouldn't be null, for its properties there are no null checks.

Then just add Objects.requireNonNull(entryChangedEvent) and remove the != null check.

Further please investigate the "call hierarchy" whether getBibEntry() may ever be null. I don't think so. Thus, please add Objects.requireNonNull(entryChangedEvent.getBibEntry();.

Is it not more useful to add the Objects.requireNonNull to the constructor of the EntryEvent?

The more places the better

tobiasdiez · 2016-09-15T21:32:21Z

src/main/java/net/sf/jabref/logic/citationstyle/CitationStyleGenerator.java

+            LOGGER.error("Bad character inside BibEntry", e);
+            // sadly one can not easily retrieve the bad char from the TokenMgrError
+            return  new StringBuilder()
+                    .append(Localization.lang("Bad character inside BibEntry"))


Use a proper word instead of BibEntry (no idea what we usually use...probably entry)

entrywas used in other translations

Just look at other localizations.

Please write cannot

tobiasdiez · 2016-09-15T21:34:24Z

src/test/java/net/sf/jabref/logic/citationstyle/CitationStyleTest.java

+    public void getDefault() throws Exception {
+        Assert.assertNotNull(CitationStyle.getDefault());
+    }
+


Please add some tests. At least one test should verify that a BibEntry is properly converted into the correct string.

I added a Test to check the generated citation with the default citation style on the test bibEntry.

oscargus · 2016-09-19T11:52:11Z

src/main/java/net/sf/jabref/gui/PreviewPanel.java

        this.basePanel = Optional.ofNullable(panel);

        createPreviewPane();
+        updateLayout();

        if (panel != null) {


Use this.basePanel instead here?

oscargus · 2016-09-19T11:57:40Z

src/main/java/net/sf/jabref/preferences/PreviewPreferences.java

+        this.preferences = preferences;
+    }
+
+    public static void putDefaults(Map<String, Object> defaults) {


Although there are some classes that use this, as far as I know, the decision is that all constants and all defaults setting should be in JabRefPreferences.

They are back in JabRefPreferences.

Olifair · 2016-09-27T11:23:08Z

Is the use of Citation Styles only for the preview pane ? I have downloaded the latest developer version and connected to a LibreOffice document to see if I could use the CSL styles to format my bibliography, but when I open the "Select style" window, only the jstyle format seems to be supported.

georgd · 2016-10-07T21:51:47Z

I think, this would be the next logical step: to use Citation Styles in Libre Office.

koppor · 2016-10-08T08:08:41Z

Please open a new issue to offer tracking the status. Maybe @oscargus has some time to work on that.

wujastyk · 2017-01-05T22:32:24Z

If I use Biblatex style biblio entries, then I have fields like "journaltitle," "location," and "date" instead of "journal," "address," and "year." These are not picked up by the new Preview citation styles.

Further, as reported by boceckts above, one can't get the citation style displayed, so as to edit it.

The whole new Preview citation feature is wonderfully helpful. I use it a lot! Thank you.

boceckts added stupro labels Sep 6, 2016

boceckts reviewed Sep 10, 2016
View reviewed changes

boceckts removed the stupro-ready-for-internal-review label Sep 10, 2016

chriba added 5 commits September 10, 2016 21:35

add CitationStyle support

8a35cb5

add missing localization

d3b8954

fix up/down button

ded0479

rename

a756c0a

add JavaDoc

321d418

koppor reviewed Sep 12, 2016
View reviewed changes

add licences

ac4f5cc

chriba mentioned this pull request Sep 12, 2016

Stable library version for citationStyles #1956

Closed

fix preview update after preferences

8392a3a

koppor reviewed Sep 12, 2016
View reviewed changes

tobiasdiez reviewed Sep 14, 2016
View reviewed changes

koppor changed the title ~~Support CitationStyles~~ upport CitationStyles Sep 15, 2016

chriba added 2 commits September 15, 2016 12:01

citationStyle cache holds only one citaitonStyle

de081a0

fixed private

3dfd3f1

koppor changed the title ~~upport CitationStyles~~ Support CitationStyles Sep 15, 2016

tobiasdiez requested changes Sep 15, 2016

View reviewed changes

fix issues

69139b5

oscargus reviewed Sep 19, 2016

View reviewed changes

chriba added 5 commits September 20, 2016 16:00

rewrite previewPreferences

96ee005

fix correct preview display after saving preferences

895414a

Merge remote-tracking branch 'jabref/master' into supportCitationStyles

aef874e

add null check to EntryEvent

5a637f3

fix typo

50c0ce5

koppor merged commit a6fe66b into JabRef:master Sep 22, 2016

chriba deleted the supportCitationStyles branch September 22, 2016 23:06

This was referenced Sep 26, 2016

Add help on Citation Styles JabRef/user-documentation#93

Open

Feature request: format preview using BibLaTeX-files (*.bbx and *.cbx-files) #1339

Closed

tobiasdiez mentioned this pull request Sep 27, 2016

Add support for CitationStyles #119

Closed

georgd mentioned this pull request Oct 9, 2016

Support Citation Style Language (CSL) Styles in LO/OO #2146

Closed

zesaro pushed a commit to zesaro/jabref that referenced this pull request Nov 22, 2016

Support CitationStyles (JabRef#1928)

d4323a5

koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 6, 2019

This was referenced Oct 6, 2019

Check biblatex -> bibtex converter JabRef/jabref-koppor#349

Closed

Quick fix CSL errors #5389

Merged

		import org.apache.commons.logging.LogFactory;


		public class CitationStyleWorker extends SwingWorker<String, Void> {

Support CitationStyles #1928

Support CitationStyles #1928

Conversation

chriba commented Sep 6, 2016 • edited Loading

Preview before:

Preview after:

Preferences before:

Preferences after:

boceckts commented Sep 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boceckts commented Sep 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boceckts commented Sep 10, 2016 • edited Loading

boceckts commented Sep 10, 2016

boceckts commented Sep 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boceckts commented Sep 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriba commented Sep 12, 2016

koppor commented Sep 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez Sep 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriba Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Olifair commented Sep 27, 2016

georgd commented Oct 7, 2016

koppor commented Oct 8, 2016

wujastyk commented Jan 5, 2017

chriba commented Sep 6, 2016 •

edited

Loading

boceckts commented Sep 10, 2016 •

edited

Loading

koppor commented Sep 12, 2016 •

edited

Loading

tobiasdiez Sep 15, 2016 •

edited

Loading

chriba Sep 19, 2016 •

edited

Loading