Skip to content
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.

Sign up for GitHub

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

Export OO/LO citations to new database #1630

Merged
merged 5 commits into from
Jul 29, 2016
Merged

Export OO/LO citations to new database #1630

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7754ee8
Export OO/LO citations to new database
oscargus Jul 26, 2016
e30fa1c
Fixed problem with duplicates
oscargus Jul 26, 2016
8b607c8
Added some comments
oscargus Jul 26, 2016
a4201f3
Fixed spelling in comment
oscargus Jul 26, 2016
fba22d9
Removed general Exception
oscargus Jul 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- Added ISBN integrity checker
- Added filter to not show selected integrity checks
- Enhance the entry customization dialog to give better visual feedback
- It is now possible to generate a new BIB database from the citations in an OpenOffice/LibreOffice document
- The arXiv fetcher now also supports free-text search queries
- [#1345](https://github.com/JabRef/jabref/issues/1345) Cleanup ISSN

Expand Down
28 changes: 28 additions & 0 deletions src/main/java/net/sf/jabref/gui/openoffice/OOBibBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import net.sf.jabref.model.database.BibDatabase;
import net.sf.jabref.model.entry.BibEntry;
import net.sf.jabref.model.entry.FieldName;
import net.sf.jabref.model.entry.IdGenerator;

import com.sun.star.awt.Point;
import com.sun.star.beans.IllegalTypeException;
Expand Down Expand Up @@ -1296,4 +1297,31 @@ public int hashCode() {

}


public BibDatabase generateDatabase(List<BibDatabase> databases, OOBibStyle style)
throws NoSuchElementException, WrappedTargetException {
BibDatabase resultDatabase = new BibDatabase();
List<String> cited = findCitedKeys();
for (String key : cited) {
for (BibDatabase loopDatabase : databases) {
Optional<BibEntry> entry = loopDatabase.getEntryByKey(key);
if (entry.isPresent()) {
BibEntry clonedEntry = (BibEntry) entry.get().clone();
clonedEntry.setId(IdGenerator.next());
resultDatabase.insertEntry(clonedEntry);
entry.get().getFieldOptional(FieldName.CROSSREF).ifPresent(crossref -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Avoid checking every optional for isPresent, use a flatMap for chained optionals:
(Optional FlatMap example)
http://codingjunkie.net/working-with-java8-optionals/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had some plans to break the loop once the key was found. In that case one is stuck with isPresent as far as I know.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The break solved the problem with the multiple entries. I was really confused for a while, but since the function looks through all tabs, it found the first generated database and included those entries once more and so on... :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. This whole loop with all the presents looked very complex and confusing at the first moment. 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments. Most of the time I like Optional (even though I'm at the best is using ifPresent), but the lack of if ... else ... and the fact that the lambda becomes its own function (which is very natural, but still prohibits writing some things, like return or break) is a bit annoying.

if (!resultDatabase.getEntryByKey(crossref).isPresent()) {
Optional<BibEntry> refEntry = loopDatabase.getEntryByKey(crossref);
if (refEntry.isPresent()) {
resultDatabase.insertEntry(refEntry.get());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I would use ifPresent(resultDatabase.insertEntry)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Copied the code from the AUX parser where more things were done in the corresponding place and didn't reflect on it.

}
}
});
}
}
}

return resultDatabase;
}

}
48 changes: 46 additions & 2 deletions src/main/java/net/sf/jabref/gui/openoffice/OpenOfficePanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import javax.swing.JRadioButtonMenuItem;
import javax.swing.JTextField;

import net.sf.jabref.BibDatabaseContext;
import net.sf.jabref.Defaults;
import net.sf.jabref.Globals;
import net.sf.jabref.gui.BasePanel;
import net.sf.jabref.gui.IconTheme;
Expand All @@ -64,7 +66,9 @@
import net.sf.jabref.logic.openoffice.UndefinedParagraphFormatException;
import net.sf.jabref.logic.util.OS;
import net.sf.jabref.model.database.BibDatabase;
import net.sf.jabref.model.database.BibDatabaseMode;
import net.sf.jabref.model.entry.BibEntry;
import net.sf.jabref.preferences.JabRefPreferences;

import com.jgoodies.forms.builder.ButtonBarBuilder;
import com.jgoodies.forms.builder.FormBuilder;
Expand Down Expand Up @@ -97,6 +101,7 @@ public class OpenOfficePanel extends AbstractWorker {
private final JButton update;
private final JButton merge = new JButton(Localization.lang("Merge citations"));
private final JButton manageCitations = new JButton(Localization.lang("Manage citations"));
private final JButton exportCitations = new JButton(Localization.lang("Generate new BIB database"));
private final JButton settingsB = new JButton(Localization.lang("Settings"));
private final JButton help = new HelpAction(Localization.lang("OpenOffice/LibreOffice integration"),
HelpFile.OPENOFFICE_LIBREOFFICE).getHelpButton();
Expand Down Expand Up @@ -281,6 +286,8 @@ public void actionPerformed(ActionEvent e) {
}
});

exportCitations.addActionListener(event -> exportEntries());

selectDocument.setEnabled(false);
pushEntries.setEnabled(false);
pushEntriesInt.setEnabled(false);
Expand All @@ -289,9 +296,11 @@ public void actionPerformed(ActionEvent e) {
update.setEnabled(false);
merge.setEnabled(false);
manageCitations.setEnabled(false);
exportCitations.setEnabled(false);
diag = new JDialog((JFrame) null, "OpenOffice/LibreOffice panel", false);

FormBuilder mainBuilder = FormBuilder.create().layout(new FormLayout("fill:pref:grow", "p,p,p,p,p,p,p,p,p,p"));
FormBuilder mainBuilder = FormBuilder.create()
.layout(new FormLayout("fill:pref:grow", "p,p,p,p,p,p,p,p,p,p,p"));

FormBuilder topRowBuilder = FormBuilder.create()
.layout(new FormLayout("fill:pref:grow, 1dlu, fill:pref:grow, 1dlu, fill:pref:grow, "
Expand All @@ -309,7 +318,8 @@ public void actionPerformed(ActionEvent e) {
mainBuilder.add(pushEntriesEmpty).xy(1, 6);
mainBuilder.add(merge).xy(1, 7);
mainBuilder.add(manageCitations).xy(1, 8);
mainBuilder.add(settingsB).xy(1, 9);
mainBuilder.add(exportCitations).xy(1, 9);
mainBuilder.add(settingsB).xy(1, 10);

JPanel content = new JPanel();
comp.setContentContainer(content);
Expand All @@ -322,6 +332,39 @@ public void actionPerformed(ActionEvent e) {

}

private void exportEntries() {
try {
if (style == null) {
style = loader.getUsedStyle();
} else {
style.ensureUpToDate();
}

ooBase.updateSortedReferenceMarks();

List<BibDatabase> databases = getBaseList();
List<String> unresolvedKeys = ooBase.refreshCiteMarkers(databases, style);
BibDatabase newDatabase = ooBase.generateDatabase(databases, style);
if (!unresolvedKeys.isEmpty()) {
JOptionPane.showMessageDialog(frame,
Localization.lang(
"Your OpenOffice/LibreOffice document references the BibTeX key '%0', which could not be found in your current database.",
unresolvedKeys.get(0)),
Localization.lang("Unable to generate new database"), JOptionPane.ERROR_MESSAGE);
}

Defaults defaults = new Defaults(
BibDatabaseMode.fromPreference(Globals.prefs.getBoolean(JabRefPreferences.BIBLATEX_DEFAULT_MODE)));

BibDatabaseContext databaseContext = new BibDatabaseContext(newDatabase, defaults);
this.frame.addTab(databaseContext, true);

} catch (Exception e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you specify a more concrete type of exception here. Catching Exception itself is not really good practice as you will end up catching programming errors such as NullPointerException as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that refreshCiteMarkers throws Exception because refreshCiteMarkersInternal throws Exception because insertReferenceMark throws Exception because com.sun.star.lang.XMultiServiceFactory.createInstance throws Exception.

I have earlier tried to specify the exceptions in OO/LO where possible, but there are still some methods that only throw Exception. It would be possible to borrow the code from the update action:

                } catch (UndefinedCharacterFormatException ex) {
                    reportUndefinedCharacterFormat(ex);
                } catch (UndefinedParagraphFormatException ex) {
                    reportUndefinedParagraphFormat(ex);
                } catch (ConnectionLostException ex) {
                    showConnectionLostErrorMessage();
                } catch (IOException ex) {
                    JOptionPane.showMessageDialog(frame,
                            Localization
                                    .lang("You must select either a valid style file, or use one of the default styles."),
                            Localization.lang("No valid style file defined"), JOptionPane.ERROR_MESSAGE);
                    LOGGER.warn("Problem with style file", ex);
                } catch (BibEntryNotFoundException ex) {
                    JOptionPane.showMessageDialog(frame,
                            Localization.lang(
                                    "Your OpenOffice/LibreOffice document references the BibTeX key '%0', which could not be found in your current database.",
                                    ex.getBibtexKey()),
                            Localization.lang("Unable to synchronize bibliography"), JOptionPane.ERROR_MESSAGE);
                    LOGGER.debug("BibEntry not found", ex);
                } catch (Exception ex) {
                    LOGGER.warn("Could not update bibliography", ex);
                }

which probably is slightly better.

The main "problem" is that there is very limited exception handling locally in the OO/LO part. Everything is propagated to the calling function. Which is good from some perspectives but makes it hard to change some things as you do not know what is actually happening and how it affect things. (Unless you really understand the code...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option would be to convert the Exception thrown by com.sun.star.lang.XMultiServiceFactory.createInstance into a custom JabRefException (the class does already exist) and check for that. This preserves stack trace and message of the original exception and is as simple as:

catch(Exception e) {
  throw new JabRefException(e);
}

Then you can only deal with JabRefException from that point on and avoid accidentally catching another one. It also doesn't clutter the code with handling code. You can do the handling as you did before, just on a more limited type of exception. Would this be an option for the OO/LO part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there are quite a few Exception types already defined in OO/LO so defining one more (in case we want JabRefException to be used later for something else) shouldn't be a problem. It may be a problem naming it since I'm not really sure what the errors are, maybe ThingsWentWrongException? ;-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, there are indeed quite some customn exceptions. But I don't understand why you want to define yet another exception? Why not just use net.sf.jabref.JabRefException

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced CreationException... One should probably "group" the exceptions slightly earlier and not just propagate them, but at least there are no catch (Exception ex) in the OO/LO part of the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, then this is good enough! I'll merge now.

LOGGER.warn("Problem generating new database.", e);
}

}

private List<BibDatabase> getBaseList() {
List<BibDatabase> databases = new ArrayList<>();
if (preferences.useAllDatabases()) {
Expand Down Expand Up @@ -418,6 +461,7 @@ private void connect(boolean auto) {
update.setEnabled(true);
merge.setEnabled(true);
manageCitations.setEnabled(true);
exportCitations.setEnabled(true);

} catch (UnsatisfiedLinkError e) {
LOGGER.warn("Could not connect to running OpenOffice/LibreOffice", e);
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_da.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1714,3 +1714,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_de.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2433,3 +2433,6 @@ Error_occured_while_executing_the_command_\"%0\".=Während_der_Ausführung_des_B

Reformat_ISSN=


Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2273,6 +2273,9 @@ Latest_version=Latest_version
Online_help_forum=Online_help_forum
Custom=Custom

Generate_new_BIB_database=Generate_new_BIB_database
Unable_to_generate_new_database=Unable_to_generate_new_database

Open_console=Open_console
Use_default_terminal_emulator=Use_default_terminal_emulator
Execute_command=Execute_command
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_es.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1616,3 +1616,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_fa.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2402,3 +2402,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_fr.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,3 +1660,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_in.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1635,3 +1635,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_it.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1735,3 +1735,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_ja.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2380,3 +2380,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_nl.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2411,3 +2411,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_no.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2807,3 +2807,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_pt_BR.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1629,3 +1629,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_ru.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2379,3 +2379,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_sv.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1572,3 +1572,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_tr.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,3 +1648,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_vi.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2403,3 +2403,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_zh.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1642,3 +1642,6 @@ Executing_command_\"%0\"...=
Error_occured_while_executing_the_command_\"%0\".=

Reformat_ISSN=

Generate_new_BIB_database=
Unable_to_generate_new_database=