Improve entrytype customization by matthiasgeiger · Pull Request #2331 · JabRef/jabref

matthiasgeiger · 2016-12-02T11:16:31Z

Goals:

… entries - fixes #2261

…ialogs

matthiasgeiger · 2016-12-11T21:41:47Z

Okay... I think the current state is now a good improvement of the old status-quo.
At the moment the prefs migration and a dialog showing a warning if customized entries would be overridden is still missing.

Apart from that I would appreciate some feedback ;-)

lenhard

I just looked through the code, but did not test this in a running JabRef. There are a number of things that can be improved quality/architecture-wise, but none of these is a blocker. So please fix what you agree with.

lenhard · 2016-12-12T13:05:01Z

-            sb.append(type).append(", ");
+    public void performAction(BasePanel panel, ParserResult parserResult) {
+
+        BibDatabaseMode mode = parserResult.getMetaData().getMode().orElse(Globals.prefs.getDefaultBibDatabaseMode());


To avoid writing that method chain multiple times in this class, maybe extract a helper method?

lenhard · 2016-12-12T13:05:52Z

+        List<EntryType> newTypes = new ArrayList<>();
+        List<EntryType> differentCustomizations = new ArrayList<>();
+
+        for(EntryType customType : allCustomizedEntryTypes) {


please adhere to the formatting guidelines: for ( and below if (

lenhard · 2016-12-12T13:07:04Z

+        JPanel checkboxPanel = new JPanel();
+        checkboxPanel.setLayout(new BoxLayout(checkboxPanel, BoxLayout.PAGE_AXIS));
+
+        JLabel customFoundLabel = new JLabel(Localization.lang("Custom entry types found in file")+".");


Also formatting problems here. I should be sufficient to auto-format the file.

lenhard · 2016-12-12T13:08:13Z

                menu.add(new ChangeTypeAction(type, panel));
            }
+
+            List<EntryType> customTypes = EntryTypes.getAllCustomTypes(BibDatabaseMode.BIBLATEX);


Would it be possible to extract the following code in a private method? It is duplicated below and the only difference is the mode.

I would prefer to leave it as it, as the helper method would require three params (BibDatabaseMode, Menu and BasePanel) for just a simple get, if, create...

lenhard · 2016-12-12T13:08:41Z

+        if (frame.getCurrentBasePanel().getBibDatabaseContext().isBiblatexMode()) {
+            panel.add(createEntryGroupPanel("BibLateX", BibLatexEntryTypes.ALL));
+
+            List<EntryType> customTypes = EntryTypes.getAllCustomTypes(BibDatabaseMode.BIBLATEX);


Would it be possible to extract the following code in a private method? It is duplicated below and the only difference is the mode.

Same as above - helper method would be overkill, imho

lenhard · 2016-12-12T13:11:02Z

     * Load all custom entry types from preferences. This method is
     * called from JabRef when the program starts.
     */
    public static void loadCustomEntryTypes(JabRefPreferences prefs) {


Could you change this class to be independent of JabRefPreferences? This would get us one step closer to our desired architecture. For the load method, this is easy (just put in CustomEntryType instead of the preferences). For the saving it is a little more tricky and you will need to push code into the calling UI-class.

Mhmmm... This is basically just a UI-Helper class. loadCustomEntryTypes is called from the JabRefMain and the CLI Argument processer.

Storage is also used in the UI only...

So would it be possible and sensible to move it as is into the gui package?

Not entirely, unfortunately. See: https://github.com/JabRef/jabref/wiki/High-Level-Documentation
The CLI is not supposed to call the gui, so anything accessed from GUI and CLI should be in logic, but it should be independent of the preferences. You could move the save and leave the load for instance.

No we just need a new package net.sf.ui which could be called by gui and cli!

... just kidding 😉 I'll think about it...

lenhard · 2016-12-12T13:11:56Z

+     */
+    public static List<EntryType> getAllCustomTypes(BibDatabaseMode mode) {
+        Collection<EntryType> allTypes = getAllValues(mode);
+        if(mode == BibDatabaseMode.BIBTEX) {


Can you extract the almost duplicated lambda into a private helper method?

Sorry, but I cannot come up with an intelligent solution for such a helper method which is not basically the same as I already wrote here 😉

Sure, no problem. This is just a suggestion.

lenhard · 2016-12-12T13:12:22Z

+    }
+
+    public static List<EntryType> getAllModifiedStandardTypes(BibDatabaseMode mode) {
+        if (mode == BibDatabaseMode.BIBTEX) {


Again, can the lambda be extracted into a helper method?

lenhard · 2016-12-12T13:13:02Z

+     */
+    private static boolean checkEqualityOfLists(List<String> fieldList1, List<String> fieldList2) {
+
+        if(fieldList1.size()!=fieldList2.size()) {


Please format this properly.

…ndard equals of List is possible

Conflicts: src/main/java/net/sf/jabref/model/EntryTypes.java src/main/java/net/sf/jabref/preferences/JabRefPreferences.java

matthiasgeiger · 2016-12-13T11:21:01Z

Apart from a CHANGELOG and some more tests this is good to go from my side.

matthiasgeiger · 2016-12-15T10:17:42Z

@mlep @frithnanth It is not yet decided whether we will include this improvement in 3.8 or not. However, there are three more translations which are added in this pull request. Could you provide me translations for these three strings? Then I'll add them manually.
Thanks!

Select_all_customized_types_to_be_stored_in_local_preferences=
Currently_unknown=
Different_Customization,_current_settings_will_be_overwritten=

frithnanth · 2016-12-15T12:07:02Z

On Thu, Dec 15, 2016 at 11:17 AM, Matthias Geiger ***@***.***> wrote: @mlep <https://github.com/mlep> @frithnanth <https://github.com/frithnanth> It is not yet decided whether we will include this improvement in 3.8 or not. However, there are three more translations which are added in this pull request. Could you provide me translations for these three strings? Then I'll add them manually. Thanks! Select_all_customized_types_to_be_stored_in_local_preferences= Select_all_customized_types_to_be_stored_in_local_

preferences=Seleziona_tutti_i_tipi_personalizzati_da_registrare_nelle_preferenze_locali

Currently_unknown= Currently_unknown=Attualmente_sconosciuto

That's the singular mode, the plural is "Attualmente_sconosciuti". But since Italian is profoundly evil, that's OK if referred to a masculine noun, the feminine forms are different, but I'm digressing :-)

Different_Customization,_current_settings_will_be_overwritten= Different_Customization,_current_settings_will_be_

overwritten=Personalizzazione_differente,_i_parametri_correnti_saranno_sovrascritti

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2331 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADyA2AnC2wjebXAMoi4YuB2CzYqky1Bgks5rIRPHgaJpZM4LCcrA> .

-- Fernando Santagata

matthiasgeiger · 2016-12-16T20:30:45Z

Thank you @frithnanth !

frithnanth · 2016-12-16T21:52:04Z

You're welcome!

…

On Fri, Dec 16, 2016 at 9:30 PM, Matthias Geiger ***@***.***> wrote: Thank you @frithnanth <https://github.com/frithnanth> ! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2331 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADyA2EhYM65slWyTwdWTJwH8EDTLmMC4ks5rIvT2gaJpZM4LCcrA> .

-- Fernando Santagata

koppor · 2017-08-18T19:49:10Z

How did we proceed regarding standard types? Is it a v4.0 issue? See JabRef#248

matthiasgeiger force-pushed the improve-entrytype-customization branch from aa3c3b2 to 34f5f29 Compare December 4, 2016 21:50

matthiasgeiger added 6 commits December 5, 2016 14:20

entry type customization based on BibdatabaseMode

60db50a

Remove unnecessary help method in BibDatabaseMode

cbbdd67

use database mode of current DB

4548190

add new storage of custom entry types

ab10ab1

load and store databasemode specific lists of custom types

830f59c

rework to use database mode specific custom types

0d45001

matthiasgeiger force-pushed the improve-entrytype-customization branch from 46772ba to 0d45001 Compare December 5, 2016 13:20

matthiasgeiger added 6 commits December 5, 2016 15:12

reset of preferences now also clears bibtexKeyPatterns and customized…

d144954

… entries - fixes #2261

reenable ignored test

9d275bc

rewrite and add additional tests to EntryTypesTestBibtex

379a398

cleanup after tests

7dafccf

add convenience method to get IeeeTranEntryType by name

e189fd7

Filter customizations of standard types in NewEntry/ChangeEntryType d…

cd13456

…ialogs

matthiasgeiger added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 11, 2016

Improve customEntryType import dialog

441964d

This was referenced Dec 12, 2016

Refactor entry type/custom entry type logic #366

Closed

Save custom entry types in bib file #365

Closed

update localization

1cc3493

lenhard approved these changes Dec 12, 2016

View reviewed changes

matthiasgeiger added 9 commits December 12, 2016 21:11

minor cleanup and rename

6d7afc1

do not cycle through all entries but only through changed ones

4c6aa25

improve naming

5aae2f9

different order of fields should indicate different types - using sta…

92c5712

…ndard equals of List is possible

incorporate feedback

4ba2c1f

fix codacy

283401f

migrate customEntryType preferences

9678cba

Merge branch 'master' into improve-entrytype-customization

fbe70d7

Conflicts: src/main/java/net/sf/jabref/model/EntryTypes.java src/main/java/net/sf/jabref/preferences/JabRefPreferences.java

fix localization duplicates

999e493

matthiasgeiger mentioned this pull request Dec 13, 2016

Customizing the entry types freezes Jabref #2318

Closed

matthiasgeiger changed the title ~~[WIP] Improve entrytype customization~~ Improve entrytype customization Dec 13, 2016

matthiasgeiger added 4 commits December 13, 2016 14:01

test entrytypes parametrized

696bcfc

add CHANGELOG for custom type changes/fixes

8584a4d

increase coverage

e247a5d

refactor to adhere to architectural constraints

5e256f5

koppor merged commit 5e256f5 into master Dec 16, 2016

koppor deleted the improve-entrytype-customization branch December 16, 2016 15:52

lenhard mentioned this pull request Dec 16, 2016

custom entry types bug #772

Closed

koppor mentioned this pull request Feb 11, 2026

Fix duplicate entry types causing wrong fields after tab switch (#14985) #15052

Merged

5 tasks

Uh oh!

Conversation

matthiasgeiger commented Dec 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthiasgeiger commented Dec 11, 2016

Uh oh!

lenhard left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthiasgeiger commented Dec 13, 2016

Uh oh!

matthiasgeiger commented Dec 15, 2016

Uh oh!

frithnanth commented Dec 15, 2016 via email

Uh oh!

matthiasgeiger commented Dec 16, 2016

Uh oh!

frithnanth commented Dec 16, 2016 via email

Uh oh!

koppor commented Aug 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matthiasgeiger commented Dec 2, 2016 •

edited

Loading