-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
CSL4LibreOffice - H [Non-bibliographic styles] #12996
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
Conversation
… new tests, collapse redundant methods. Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
This reverts commit d8eff92.
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
INTERNAL_STYLES.add(style); | ||
} | ||
} catch (IOException e) { | ||
LOGGER.error("Error loading style file: {}", path, e); | ||
styleCount--; | ||
} | ||
} | ||
LOGGER.info("Loaded {} CSL styles", styleCount); | ||
LOGGER.info("Loaded {} CSL styles", styleCount); |
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.
The logging statement should not be reformatted unless new statements are added. The patch only changes indentation without adding new 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.
It corrects the indent
*/ | ||
public record StyleInfo(String title, boolean isNumericStyle) { | ||
public record StyleInfo(String title, boolean isNumericStyle, boolean hasBibliography) { |
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.
The JavaDoc for the StyleInfo record was updated to include 'hasBibliography', but the JavaDoc for the createCitationStyleFromSource method was not updated to reflect the change in the CitationStyle constructor.
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.
No need
src/main/java/org/jabref/logic/citationstyle/CitationStyleCatalogGenerator.java
Show resolved
Hide resolved
src/main/java/org/jabref/logic/citationstyle/CitationStyleGenerator.java
Show resolved
Hide resolved
@@ -18,6 +18,10 @@ public CitationStylePreviewLayout(CitationStyle citationStyle, BibEntryTypesMana | |||
|
|||
@Override | |||
public String generatePreview(BibEntry entry, BibDatabaseContext databaseContext) { | |||
if (!citationStyle.hasBibliography()) { |
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.
The patch introduces a duplicate check for 'citationStyle.hasBibliography()' which is already present in the existing code. This results in code duplication and should be refactored to avoid redundancy.
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.
It's just for conservative safety.
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
@Test | ||
void discoverInternalCitationStylesNotNull() { | ||
List<CitationStyle> styleList = CSLStyleLoader.getInternalStyles(); | ||
assertNotNull(styleList); |
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.
Maybe, assertNotEquals(List.of(), styleList
to check that the list has some contents?
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.
Good idea, done
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 think assertFalse(styleList.isEmpty())
is equivalent for more brevity, changed to it
public static String getBibliographyTitle() { | ||
return bibliographyTitle; | ||
} | ||
|
||
public static String getBibliographyHeaderFormat() { | ||
return bibliographyHeaderFormat; | ||
} | ||
private static final Pattern YEAR_IN_CITATION_PATTERN = Pattern.compile("(.)(.*), (\\d{4}.*)"); | ||
|
||
public static void setBibliographyProperties(OpenOfficePreferences openOfficePreferences) { | ||
bibliographyTitle = openOfficePreferences.getCslBibliographyTitle(); | ||
bibliographyHeaderFormat = openOfficePreferences.getCslBibliographyHeaderFormat(); |
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.
Removal as an outcome of discussion at https://github.com/JabRef/jabref/pull/12784/files#r2058893104
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
src/test/java/org/jabref/logic/citationstyle/CitationStyleGeneratorTest.java
Show resolved
Hide resolved
src/test/java/org/jabref/logic/citationstyle/CitationStyleGeneratorTest.java
Show resolved
Hide resolved
src/test/java/org/jabref/logic/citationstyle/CitationStyleGeneratorTest.java
Show resolved
Hide resolved
src/test/java/org/jabref/logic/citationstyle/CitationStyleGeneratorTest.java
Show resolved
Hide resolved
src/test/java/org/jabref/logic/citationstyle/CitationStyleGeneratorTest.java
Show resolved
Hide resolved
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
@trag-bot didn't find any issues in the code! ✅✨ |
1 similar comment
@trag-bot didn't find any issues in the code! ✅✨ |
JabRef/jabref#12996 src/test/java/org/jabref/logic/openoffice/oocsltext/CSLFormatUtilsTest.java
Follow-up to #12951
Fixes #12990 (comment)
Support for non-bibliographic styles
Certain styles specified in our internal
.csl
files had a missing "bibliography" format specification section. Turns out there are styles that specify only citation formatting without bibliography.[Fun fact: Some of these styles happen to be "note" citation styles (characterized by
citation-format="note"
in the source)].This PR introduces support for such styles, thus enabling support for 82 new styles like "Oxford Art Journal", "Journal of Law, Medicine & Ethics", "Australian Historical Studies", "Business and Human Rights Journal", "Bluebook Law Review", "Journal of Applied Philosophy", "The Journal of Clinical Ethics", "South African Law Journal", "International Review of the Red Cross" and so on.
We now support a total of 2680 built-in styles.
The entire list of additions can be found below:
Note:
Make/Sync bibliography
button will be disabled when such a style is selected.Other improvements
CitationStyleGenerator
.CitationStyleCatalogGenerator
.Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if change is visible to the user)