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.

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

Migrate Review field in entry preview to comment #4100

Merged
merged 10 commits into from
Jun 13, 2018
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We reworked the "Edit file" dialog to make it resizeable and improved the workflow for adding and editing files https://github.com/JabRef/jabref/issues/2970
- We fixed an issue where the month was not shown in the preview https://github.com/JabRef/jabref/issues/3239.
- Rewritten logic to detect a second jabref instance. [#4023](https://github.com/JabRef/jabref/issues/4023)
- We fixed an issue where the default entry preview style still contained the field `review`. The field `review` in the style is now replaced with comment to be consistent with the entry editor [#4098](https://github.com/JabRef/jabref/issues/4098)

### Removed
- The feature to "mark entries" was removed and merged with the groups functionality. For migration, a group is created for every value of the `__markedentry` field and the entry is added to this group.
Expand Down
63 changes: 36 additions & 27 deletions src/main/java/org/jabref/migrations/PreferencesMigrations.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.slf4j.LoggerFactory;

public class PreferencesMigrations {

private static final Logger LOGGER = LoggerFactory.getLogger(PreferencesMigrations.class);

private PreferencesMigrations() {
Expand All @@ -40,6 +41,7 @@ public static void runMigrations() {
upgradeKeyBindingsToJavaFX();
addCrossRefRelatedFieldsForAutoComplete();
upgradeObsoleteLookAndFeels();
upgradePreviewStyleFromReviewToComment();
}

/**
Expand Down Expand Up @@ -157,7 +159,7 @@ private static void upgradeStoredCustomEntryTypes() {

try {
if (mainPrefsNode.nodeExists(JabRefPreferences.CUSTOMIZED_BIBTEX_TYPES) ||
mainPrefsNode.nodeExists(JabRefPreferences.CUSTOMIZED_BIBLATEX_TYPES)) {
mainPrefsNode.nodeExists(JabRefPreferences.CUSTOMIZED_BIBLATEX_TYPES)) {
// skip further processing as prefs already have been migrated
} else {
LOGGER.info("Migrating old custom entry types.");
Expand Down Expand Up @@ -213,21 +215,21 @@ private static void migrateFileImportPattern(String oldStylePattern, String newS
String preferenceFileNamePattern = mainPrefsNode.get(JabRefPreferences.IMPORT_FILENAMEPATTERN, null);

if ((preferenceFileNamePattern != null) &&
oldStylePattern.equals(preferenceFileNamePattern)) {
oldStylePattern.equals(preferenceFileNamePattern)) {
// Upgrade the old-style File Name pattern to new one:
mainPrefsNode.put(JabRefPreferences.IMPORT_FILENAMEPATTERN, newStylePattern);
LOGGER.info("migrated old style " + JabRefPreferences.IMPORT_FILENAMEPATTERN +
" value \"" + oldStylePattern + "\" to new value \"" +
newStylePattern + "\" in the preference file");
" value \"" + oldStylePattern + "\" to new value \"" +
newStylePattern + "\" in the preference file");

if (prefs.hasKey(JabRefPreferences.IMPORT_FILENAMEPATTERN)) {
// Update also the key in the current application settings, if necessary:
String fileNamePattern = prefs.get(JabRefPreferences.IMPORT_FILENAMEPATTERN);
if (oldStylePattern.equals(fileNamePattern)) {
prefs.put(JabRefPreferences.IMPORT_FILENAMEPATTERN, newStylePattern);
LOGGER.info("migrated old style " + JabRefPreferences.IMPORT_FILENAMEPATTERN +
" value \"" + oldStylePattern + "\" to new value \"" +
newStylePattern + "\" in the running application");
" value \"" + oldStylePattern + "\" to new value \"" +
newStylePattern + "\" in the running application");
}
}
}
Expand All @@ -242,10 +244,10 @@ static void upgradeImportFileAndDirePatterns() {
// Check for prefs node for Version <= 4.0
if (mainPrefsNode.get(JabRefPreferences.IMPORT_FILENAMEPATTERN, null) != null) {

String[] oldStylePatterns = new String[]{"\\bibtexkey",
"\\bibtexkey\\begin{title} - \\format[RemoveBrackets]{\\title}\\end{title}"};
String[] newStylePatterns = new String[]{"[bibtexkey]",
"[bibtexkey] - [fulltitle]"};
String[] oldStylePatterns = new String[] {"\\bibtexkey",
"\\bibtexkey\\begin{title} - \\format[RemoveBrackets]{\\title}\\end{title}"};
String[] newStylePatterns = new String[] {"[bibtexkey]",
"[bibtexkey] - [fulltitle]"};
for (int i = 0; i < oldStylePatterns.length; i++) {
migrateFileImportPattern(oldStylePatterns[i], newStylePatterns[i], prefs, mainPrefsNode);
}
Expand Down Expand Up @@ -281,11 +283,11 @@ private static void addCrossRefRelatedFieldsForAutoComplete() {
}

private static void migrateTypedKeyPrefs(JabRefPreferences prefs, Preferences oldPatternPrefs)
throws BackingStoreException {
throws BackingStoreException {
LOGGER.info("Found old Bibtex Key patterns which will be migrated to new version.");

GlobalBibtexKeyPattern keyPattern = GlobalBibtexKeyPattern.fromPattern(
prefs.get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN));
prefs.get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN));
for (String key : oldPatternPrefs.keys()) {
keyPattern.addBibtexKeyPattern(key, oldPatternPrefs.get(key, null));
}
Expand All @@ -297,20 +299,27 @@ private static void upgradeObsoleteLookAndFeels() {
String currentLandF = prefs.get(JabRefPreferences.WIN_LOOK_AND_FEEL);

Stream.of("com.jgoodies.looks.windows.WindowsLookAndFeel", "com.jgoodies.looks.plastic.PlasticLookAndFeel",
"com.jgoodies.looks.plastic.Plastic3DLookAndFeel", "com.jgoodies.looks.plastic.PlasticXPLookAndFeel",
"com.sun.java.swing.plaf.gtk.GTKLookAndFeel")
.filter(style -> style.equals(currentLandF))
.findAny()
.ifPresent(loolAndFeel -> {
if (OS.WINDOWS) {
String windowsLandF = "com.sun.java.swing.plaf.windows.WindowsLookAndFeel";
prefs.put(JabRefPreferences.WIN_LOOK_AND_FEEL, windowsLandF);
LOGGER.info("Switched from obsolete look and feel " + currentLandF + " to " + windowsLandF);
} else {
String nimbusLandF = "javax.swing.plaf.nimbus.NimbusLookAndFeel";
prefs.put(JabRefPreferences.WIN_LOOK_AND_FEEL, nimbusLandF);
LOGGER.info("Switched from obsolete look and feel " + currentLandF + " to " + nimbusLandF);
}
});
"com.jgoodies.looks.plastic.Plastic3DLookAndFeel", "com.jgoodies.looks.plastic.PlasticXPLookAndFeel",
"com.sun.java.swing.plaf.gtk.GTKLookAndFeel")
.filter(style -> style.equals(currentLandF))
.findAny()
.ifPresent(loolAndFeel -> {
if (OS.WINDOWS) {
String windowsLandF = "com.sun.java.swing.plaf.windows.WindowsLookAndFeel";
prefs.put(JabRefPreferences.WIN_LOOK_AND_FEEL, windowsLandF);
LOGGER.info("Switched from obsolete look and feel " + currentLandF + " to " + windowsLandF);
} else {
String nimbusLandF = "javax.swing.plaf.nimbus.NimbusLookAndFeel";
prefs.put(JabRefPreferences.WIN_LOOK_AND_FEEL, nimbusLandF);
LOGGER.info("Switched from obsolete look and feel " + currentLandF + " to " + nimbusLandF);
}
});
}

static void upgradePreviewStyleFromReviewToComment() {
JabRefPreferences prefs = Globals.prefs;
Copy link
Member

Choose a reason for hiding this comment

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

You can pass the preferences class as a parameter and then use mock in combination with verify in the tests below (instead of working on the actual preference object).

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried around with this but I currently see no way to do this. When I mock the object, calling the migration method on the object will have no effect as it operates directly on the preferences object. I don't like that operating on the real prefs object but
Sure I can mock returns, but that's not the goal of this test.
However, I could remove the dependency on the actual Globals.prefs object by passing a Preferences object as parametre

String currentPreviewStyle = prefs.get(JabRefPreferences.PREVIEW_STYLE);
String migratedStyle = currentPreviewStyle.replace("\\begin{review}<BR><BR><b>Review: </b> \\format[HTMLChars]{\\review} \\end{review}", "\\begin{comment}<BR><BR><b>Comment: </b> \\format[HTMLChars]{\\comment} \\end{comment}");
prefs.put(JabRefPreferences.PREVIEW_STYLE, migratedStyle);
}
}
18 changes: 10 additions & 8 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,15 @@ public class JabRefPreferences implements PreferencesService {
// Id Entry Generator Preferences
public static final String ID_ENTRY_GENERATOR = "idEntryGenerator";

// Preview
//public because needed for pref migration
public static final String PREVIEW_STYLE = "previewStyle";
Copy link
Member

Choose a reason for hiding this comment

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

You can simply add a get/setPreviewStyle method which I would prefer.


private static final String CYCLE_PREVIEW_POS = "cyclePreviewPos";
private static final String CYCLE_PREVIEW = "cyclePreview";
private static final String PREVIEW_PANEL_HEIGHT = "previewPanelHeightFX";
private static final String PREVIEW_ENABLED = "previewEnabled";

// Auto completion
private static final String AUTO_COMPLETE = "autoComplete";
private static final String AUTOCOMPLETER_FIRSTNAME_MODE = "autoCompFirstNameMode";
Expand Down Expand Up @@ -396,13 +405,6 @@ public class JabRefPreferences implements PreferencesService {
//GroupViewMode
private static final String GROUP_INTERSECT_UNION_VIEW_MODE = "groupIntersectUnionViewModes";

// Preview
private static final String CYCLE_PREVIEW_POS = "cyclePreviewPos";
private static final String CYCLE_PREVIEW = "cyclePreview";
private static final String PREVIEW_PANEL_HEIGHT = "previewPanelHeightFX";
private static final String PREVIEW_STYLE = "previewStyle";
private static final String PREVIEW_ENABLED = "previewEnabled";

// Helper string
private static final String USER_HOME = System.getProperty("user.home");
// solves the issue java.lang.RuntimeException: Internal graphics not initialized yet
Expand Down Expand Up @@ -788,7 +790,7 @@ private JabRefPreferences() {
+ "\\begin{year}<b>\\year</b>\\end{year}\\begin{volume}<i>, \\volume</i>\\end{volume}"
+ "\\begin{pages}, \\format[FormatPagesForHTML]{\\pages} \\end{pages}__NEWLINE__"
+ "\\begin{abstract}<BR><BR><b>Abstract: </b> \\format[HTMLChars]{\\abstract} \\end{abstract}__NEWLINE__"
+ "\\begin{review}<BR><BR><b>Review: </b> \\format[HTMLChars]{\\review} \\end{review}"
+ "\\begin{comment}<BR><BR><b>Comment: </b> \\format[HTMLChars]{\\comment} \\end{comment}"
+ "</dd>__NEWLINE__<p></p></font>");

setLanguageDependentDefaultValues();
Expand Down
30 changes: 15 additions & 15 deletions src/main/resources/resource/layout/listrefs/listrefs.begin.layout
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

// Search settings
var searchAbstract = true; // search in abstract
var searchReview = true; // search in review
var searchComment = true; // search in comment

var noSquiggles = true; // ignore diacritics when searching
var searchRegExp = false; // enable RegExp searches
Expand Down Expand Up @@ -65,7 +65,7 @@ function loadTableData() {
searchTable = document.getElementById('qs_table');
var allRows = searchTable.getElementsByTagName('tbody')[0].getElementsByTagName('tr');

// split all rows into entryRows and infoRows (e.g. abstract, review, bibtex)
// split all rows into entryRows and infoRows (e.g. abstract, comment, bibtex)
entryRows = new Array(); infoRows = new Array(); absRows = new Array(); revRows = new Array();

// get data from each row
Expand All @@ -81,11 +81,11 @@ function loadTableData() {
j ++;
} else {
infoRows[k++] = allRows[i];
// check for abstract/review
// check for abstract/comment
if (allRows[i].className.match(/abstract/)) {
absRows.push(allRows[i]);
absRowsData[j-1] = stripDiacritics(getTextContent(allRows[i]));
} else if (allRows[i].className.match(/review/)) {
} else if (allRows[i].className.match(/comment/)) {
revRows.push(allRows[i]);
revRowsData[j-1] = stripDiacritics(getTextContent(allRows[i]));
}
Expand Down Expand Up @@ -141,7 +141,7 @@ function quickSearch(){
if(searchAbstract && absRowsData[i]!=undefined) {
if (absRowsData[i].search(textRegExp) != -1){ found=true; }
}
if(searchReview && revRowsData[i]!=undefined) {
if(searchComment && revRowsData[i]!=undefined) {
if (revRowsData[i].search(textRegExp) != -1){ found=true; }
}
}
Expand Down Expand Up @@ -217,8 +217,8 @@ function toggleInfo(articleid,info) {

if (abs && info == 'abstract') {
abs.className.indexOf('noshow') == -1?abs.className = 'abstract noshow':abs.className = 'abstract show';
} else if (rev && info == 'review') {
rev.className.indexOf('noshow') == -1?rev.className = 'review noshow':rev.className = 'review show';
} else if (rev && info == 'comment') {
rev.className.indexOf('noshow') == -1?rev.className = 'comment noshow':rev.className = 'comment show';
} else if (bib && info == 'bibtex') {
bib.className.indexOf('noshow') == -1?bib.className = 'bibtex noshow':bib.className = 'bibtex show';
} else {
Expand All @@ -240,12 +240,12 @@ function toggleInfo(articleid,info) {
}
}

// When there's a combination of abstract/review/bibtex showing, need to add class for correct styling
// When there's a combination of abstract/comment/bibtex showing, need to add class for correct styling
if(absshow) {
(revshow||bibshow)?abs.className = 'abstract nextshow':abs.className = 'abstract';
}
if (revshow) {
bibshow?rev.className = 'review nextshow': rev.className = 'review';
bibshow?rev.className = 'comment nextshow': rev.className = 'comment';
}

}
Expand Down Expand Up @@ -304,8 +304,8 @@ function updateSetting(obj){
searchAbstract=!searchAbstract;
redoQS();
break;
case "opt_searchRev":
searchReview=!searchReview;
case "opt_searchComment":
searchComment=!searchComment;
redoQS();
break;
case "opt_useRegExp":
Expand All @@ -322,12 +322,12 @@ function updateSetting(obj){

function initPreferences(){
if(searchAbstract){document.getElementById("opt_searchAbs").checked = true;}
if(searchReview){document.getElementById("opt_searchRev").checked = true;}
if(searchComment){document.getElementById("opt_searchComment").checked = true;}
if(noSquiggles){document.getElementById("opt_noAccents").checked = true;}
if(searchRegExp){document.getElementById("opt_useRegExp").checked = true;}

if(numAbs==0) {document.getElementById("opt_searchAbs").parentNode.style.display = 'none';}
if(numRev==0) {document.getElementById("opt_searchRev").parentNode.style.display = 'none';}
if(numRev==0) {document.getElementById("opt_searchComment").parentNode.style.display = 'none';}
}

function toggleSettings(){
Expand Down Expand Up @@ -375,7 +375,7 @@ td a:hover { text-decoration: underline; }

tr.noshow { display: none;}
tr.highlight td { background-color: #EFEFEF; border-top: 2px #2E2E2E solid; font-weight: bold; }
tr.abstract td, tr.review td, tr.bibtex td { background-color: #EFEFEF; text-align: justify; border-bottom: 2px #2E2E2E solid; }
tr.abstract td, tr.comment td, tr.bibtex td { background-color: #EFEFEF; text-align: justify; border-bottom: 2px #2E2E2E solid; }
tr.nextshow td { border-bottom-style: none; }

tr.bibtex pre { width: 100%; overflow: auto; white-space: pre-wrap;}
Expand All @@ -396,7 +396,7 @@ p.infolinks { margin: 0.3em 0em 0em 0em; padding: 0px; }
<div id="settings" class="hidden">
<ul>
<li><input type="checkbox" class="search_setting" id="opt_searchAbs" onchange="updateSetting(this)"><label for="opt_searchAbs"> include abstract</label></li>
<li><input type="checkbox" class="search_setting" id="opt_searchRev" onchange="updateSetting(this)"><label for="opt_searchRev"> include review</label></li>
<li><input type="checkbox" class="search_setting" id="opt_searchComment" onchange="updateSetting(this)"><label for="opt_searchComment"> include comment</label></li>
<li><input type="checkbox" class="search_setting" id="opt_useRegExp" onchange="updateSetting(this)"><label for="opt_useRegExp"> use RegExp</label></li>
<li><input type="checkbox" class="search_setting" id="opt_noAccents" onchange="updateSetting(this)"><label for="opt_noAccents"> ignore accents</label></li>
</ul>
Expand Down
10 changes: 5 additions & 5 deletions src/main/resources/resource/layout/listrefs/listrefs.layout
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
<tr id="\format{\bibtexkey}" class="entry">
<td>\begin{author}\format[Authors(LastFirst,NoPunc,InitialsNoSpace),HTMLChars]{\author}\end{author} (\year), <i>"\format[HTMLChars]{\title}"</i>\begin{journal}, \format[HTMLChars]{\journal}.\end{journal}\begin{booktitle}, In \format[HTMLChars]{\booktitle}.\end{booktitle}\begin{howpublished}, \howpublished.\end{howpublished}\begin{school}. Thesis at: \format[HTMLChars]{\school}.\end{school}\begin{address} \format[HTMLChars]{\address}\end{address}\begin{month}, \format[HTMLChars]{\month}, \year.\end{month} \begin{volume} Vol. \volume\end{volume}\begin{number}(\format[FormatPagesForHTML]{\number})\end{number}\begin{pages}, pp. \format[FormatPagesForHTML]{\pages}.\end{pages}\begin{publisher} \format[HTMLChars]{\publisher}.\end{publisher}
<p class="infolinks">\begin{abstract}[<a href="javascript:toggleInfo('\format{\bibtexkey}','abstract')">Abstract</a>]\end{abstract}\begin{review} [<a href="javascript:toggleInfo('\format{\bibtexkey}','review')">Review</a>] \end{review} [<a href="javascript:toggleInfo('\format{\bibtexkey}','bibtex')">BibTeX</a>]\begin{doi} [<a href="\format[DOICheck]{\doi}" target="_blank">DOI</a>]\end{doi}\begin{url} [<a href="\format{\url}" target="_blank">URL</a>]\end{url}</p>
<p class="infolinks">\begin{abstract}[<a href="javascript:toggleInfo('\format{\bibtexkey}','abstract')">Abstract</a>]\end{abstract}\begin{comment} [<a href="javascript:toggleInfo('\format{\bibtexkey}','comment')">Comment</a>] \end{comment} [<a href="javascript:toggleInfo('\format{\bibtexkey}','bibtex')">BibTeX</a>]\begin{doi} [<a href="\format[DOICheck]{\doi}" target="_blank">DOI</a>]\end{doi}\begin{url} [<a href="\format{\url}" target="_blank">URL</a>]\end{url}</p>
</td>
</tr>\begin{abstract}
<tr id="abs_\format{\bibtexkey}" class="abstract noshow">
<td><b>Abstract</b>: \format[HTMLChars]{\abstract}</td>
</tr>\end{abstract}\begin{review}
<tr id="rev_\format{\bibtexkey}" class="review noshow">
<td><b>Review</b>: \format[HTMLChars]{\review}</td>
</tr>\end{review}
</tr>\end{abstract}\begin{comment}
<tr id="rev_\format{\bibtexkey}" class="comment noshow">
<td><b>Comment</b>: \format[HTMLChars]{\comment}</td>
</tr>\end{comment}
<tr id="bib_\format{\bibtexkey}" class="bibtex noshow">
<td><b>BibTeX</b>:
<pre>
Expand Down
10 changes: 5 additions & 5 deletions src/main/resources/resource/layout/listrefs/listrefs.misc.layout
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
<tr id="\format{\bibtexkey}" class="entry">
<td>\begin{author}\format[Authors(LastFirst,InitialsNoSpace,NoPunc),HTMLChars]{\author}\end{author} (\year), <i>"\format[HTMLChars]{\title}"</i>\begin{howpublished}, \howpublished\end{howpublished}. \begin{month}\format[HTMLChars]{\month}, \year.\end{month}
<p class="infolinks">\begin{abstract}[<a href="javascript:toggleInfo('\format{\bibtexkey}','abstract')">Abstract</a>]\end{abstract}\begin{review} [<a href="javascript:toggleInfo('\format{\bibtexkey}','review')">Review</a>] \end{review} [<a href="javascript:toggleInfo('\format{\bibtexkey}','bibtex')">BibTeX</a>]\begin{doi} [<a href="\format[DOICheck]{\doi}" target="_blank">DOI</a>]\end{doi}\begin{url} [<a href="\format{\url}" target="_blank">URL</a>]\end{url}</p>
<p class="infolinks">\begin{abstract}[<a href="javascript:toggleInfo('\format{\bibtexkey}','abstract')">Abstract</a>]\end{abstract}\begin{comment} [<a href="javascript:toggleInfo('\format{\bibtexkey}','comment')">Comment</a>] \end{comment} [<a href="javascript:toggleInfo('\format{\bibtexkey}','bibtex')">BibTeX</a>]\begin{doi} [<a href="\format[DOICheck]{\doi}" target="_blank">DOI</a>]\end{doi}\begin{url} [<a href="\format{\url}" target="_blank">URL</a>]\end{url}</p>
</td>
</tr>\begin{abstract}
<tr id="abs_\format{\bibtexkey}" class="abstract noshow">
<td><b>Abstract</b>: \format[HTMLChars]{\abstract}</td>
</tr>\end{abstract}\begin{review}
<tr id="rev_\format{\bibtexkey}" class="review noshow">
<td><b>Review</b>: \format[HTMLChars]{\review}</td>
</tr>\end{review}
</tr>\end{abstract}\begin{comment}
<tr id="rev_\format{\bibtexkey}" class="comment noshow">
<td><b>Comment</b>: \format[HTMLChars]{\comment}</td>
</tr>\end{comment}
<tr id="bib_\format{\bibtexkey}" class="bibtex noshow">
<td><b>BibTeX</b>:
<pre>
Expand Down
Loading