-
-
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
Validates the file path of a TexGroup and fixes Texgroup's "Library has been modified by another program" #6586
Changes from 21 commits
23557f9
2eac649
01c36a3
44b8896
f36ece6
f554c95
53cd9ad
ad11243
d20388f
8b7063d
7b65150
5b7d80e
fe895cf
db60939
f593b13
586d55b
6a82156
f092108
74f05ea
ba367ad
c120c45
ec8933e
01cdb44
e7a3c3f
49c3a2b
26342ab
aac9ffc
f98466a
3f2f46f
5a3548a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -6,10 +6,13 @@ | |||
import java.nio.file.Path; | ||||
import java.util.ArrayList; | ||||
import java.util.Collections; | ||||
import java.util.Comparator; | ||||
import java.util.HashMap; | ||||
import java.util.List; | ||||
import java.util.Map; | ||||
import java.util.Optional; | ||||
import java.util.stream.Collectors; | ||||
import java.util.stream.Stream; | ||||
|
||||
import org.jabref.logic.cleanup.Cleanups; | ||||
import org.jabref.logic.importer.ParseException; | ||||
|
@@ -49,68 +52,62 @@ public MetaData parse(MetaData metaData, Map<String, String> data, Character key | |||
List<String> defaultCiteKeyPattern = new ArrayList<>(); | ||||
Map<EntryType, List<String>> nonDefaultCiteKeyPatterns = new HashMap<>(); | ||||
|
||||
for (Map.Entry<String, String> entry : data.entrySet()) { | ||||
// process groups (GROUPSTREE and GROUPSTREE_LEGACY) at the very end (otherwise it can happen that not all dependent data are set) | ||||
Stream<Map.Entry<String, String>> entrySetStream = data.entrySet().stream().sorted(groupsLast()); | ||||
systemoperator marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
|
||||
for (Map.Entry<String, String> entry : entrySetStream.collect(Collectors.toList())) { | ||||
List<String> value = getAsList(entry.getValue()); | ||||
|
||||
if (entry.getKey().startsWith(MetaData.PREFIX_KEYPATTERN)) { | ||||
EntryType entryType = EntryTypeFactory.parse(entry.getKey().substring(MetaData.PREFIX_KEYPATTERN.length())); | ||||
nonDefaultCiteKeyPatterns.put(entryType, Collections.singletonList(getSingleItem(value))); | ||||
continue; | ||||
} else if (entry.getKey().startsWith(MetaData.FILE_DIRECTORY + '-')) { | ||||
// The user name comes directly after "FILE_DIRECTORY-" | ||||
String user = entry.getKey().substring(MetaData.FILE_DIRECTORY.length() + 1); | ||||
metaData.setUserFileDirectory(user, getSingleItem(value)); | ||||
continue; | ||||
} else if (entry.getKey().startsWith(MetaData.SELECTOR_META_PREFIX)) { | ||||
metaData.addContentSelector(ContentSelectors.parse(FieldFactory.parseField(entry.getKey().substring(MetaData.SELECTOR_META_PREFIX.length())), StringUtil.unquote(entry.getValue(), MetaData.ESCAPE_CHARACTER))); | ||||
continue; | ||||
} else if (entry.getKey().startsWith(MetaData.FILE_DIRECTORY + "Latex-")) { | ||||
// The user name comes directly after "FILE_DIRECTORYLatex-" | ||||
String user = entry.getKey().substring(MetaData.FILE_DIRECTORY.length() + 6); | ||||
Path path = Path.of(getSingleItem(value)).normalize(); | ||||
metaData.setLatexFileDirectory(user, path); | ||||
continue; | ||||
} | ||||
|
||||
switch (entry.getKey()) { | ||||
case MetaData.GROUPSTREE: | ||||
case MetaData.GROUPSTREE_LEGACY: | ||||
metaData.setGroups(GroupsParser.importGroups(value, keywordSeparator, fileMonitor, metaData)); | ||||
break; | ||||
case MetaData.SAVE_ACTIONS: | ||||
metaData.setSaveActions(Cleanups.parse(value)); | ||||
break; | ||||
case MetaData.DATABASE_TYPE: | ||||
metaData.setMode(BibDatabaseMode.parse(getSingleItem(value))); | ||||
break; | ||||
case MetaData.KEYPATTERNDEFAULT: | ||||
defaultCiteKeyPattern = Collections.singletonList(getSingleItem(value)); | ||||
break; | ||||
case MetaData.PROTECTED_FLAG_META: | ||||
if (Boolean.parseBoolean(getSingleItem(value))) { | ||||
metaData.markAsProtected(); | ||||
} else { | ||||
metaData.markAsNotProtected(); | ||||
} | ||||
break; | ||||
case MetaData.FILE_DIRECTORY: | ||||
metaData.setDefaultFileDirectory(getSingleItem(value)); | ||||
break; | ||||
case MetaData.SAVE_ORDER_CONFIG: | ||||
metaData.setSaveOrderConfig(SaveOrderConfig.parse(value)); | ||||
break; | ||||
default: | ||||
// Keep meta data items that we do not know in the file | ||||
metaData.putUnknownMetaDataItem(entry.getKey(), value); | ||||
} else if (entry.getKey().equals(MetaData.SAVE_ACTIONS)) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we keep the You can use the new Java14 switch: https://openjdk.java.net/jeps/361 (File needs to be added to jabref/config/checkstyle/checkstyle.xml Line 15 in ca06b71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still a huge p.i.t.a. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The key problem is here that switch cases are not executed in order. That's the reason for the if else There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant only for the lower part; not with the xxxx continue statements. Just for the endless chain of There is ALWAYS This is just a matter of taste. Maybe, I'll just do it and open another PR to have the discussions there. Should not be a show stopper for us now. |
||||
metaData.setSaveActions(Cleanups.parse(value)); | ||||
} else if (entry.getKey().equals(MetaData.DATABASE_TYPE)) { | ||||
metaData.setMode(BibDatabaseMode.parse(getSingleItem(value))); | ||||
} else if (entry.getKey().equals(MetaData.KEYPATTERNDEFAULT)) { | ||||
defaultCiteKeyPattern = Collections.singletonList(getSingleItem(value)); | ||||
} else if (entry.getKey().equals(MetaData.PROTECTED_FLAG_META)) { | ||||
if (Boolean.parseBoolean(getSingleItem(value))) { | ||||
metaData.markAsProtected(); | ||||
} else { | ||||
metaData.markAsNotProtected(); | ||||
} | ||||
} else if (entry.getKey().equals(MetaData.FILE_DIRECTORY)) { | ||||
metaData.setDefaultFileDirectory(getSingleItem(value)); | ||||
} else if (entry.getKey().equals(MetaData.SAVE_ORDER_CONFIG)) { | ||||
metaData.setSaveOrderConfig(SaveOrderConfig.parse(value)); | ||||
} else if (entry.getKey().equals(MetaData.GROUPSTREE) || entry.getKey().equals(MetaData.GROUPSTREE_LEGACY)) { | ||||
metaData.setGroups(GroupsParser.importGroups(value, keywordSeparator, fileMonitor, metaData)); | ||||
} else { | ||||
// Keep meta data items that we do not know in the file | ||||
metaData.putUnknownMetaDataItem(entry.getKey(), value); | ||||
} | ||||
} | ||||
|
||||
if (!defaultCiteKeyPattern.isEmpty() || !nonDefaultCiteKeyPatterns.isEmpty()) { | ||||
metaData.setCiteKeyPattern(defaultCiteKeyPattern, nonDefaultCiteKeyPatterns); | ||||
} | ||||
|
||||
return metaData; | ||||
} | ||||
|
||||
private static Comparator<? super Map.Entry<String, String>> groupsLast() { | ||||
return (s1, s2) -> MetaData.GROUPSTREE.equals(s1.getKey()) || MetaData.GROUPSTREE_LEGACY.equals(s1.getKey()) ? 1 : | ||||
MetaData.GROUPSTREE.equals(s2.getKey()) || MetaData.GROUPSTREE_LEGACY.equals(s2.getKey()) ? -1 : 0; | ||||
} | ||||
|
||||
/** | ||||
* Returns the first item in the list. | ||||
* If the specified list does not contain exactly one item, then a {@link ParseException} will be thrown. | ||||
|
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.
@stefan-kolb Would review here: "Fail fast". Not endless nested if/else statements, but just exit early.
Therefore, I proposed:
Think, I will start another PR with this and the other code style thing and we can have the discussion there --> we need to get this fix in soon.