jabref icon indicating copy to clipboard operation
jabref copied to clipboard

Enhance customization of entry types

Open koppor opened this issue 2 years ago • 58 comments

As user, I want to create a new entry type. For example, a new entry type "Person" with "name" and "googlescholar" as fields. JabRef offers advanced field handling for names, URLs, and more. This is implemented using org.jabref.model.entry.field.FieldProperty. The current dialog enables only the addition and deletion of fields. Not assigning field properties or changing the name.

Thus, TODOs:

  • [ ] Enable setting zero ore more org.jabref.model.entry.field.FieldPropertys. Should be done in a combo-box allowing multiple selections.
  • [x] ~~Enable update of name: I cannot fix typos in the name -- it should be possible to double click the name and then change the value:~~
  • [x] ~~Optional: Enable casing: I tried to add "GoogleScholar", but I could not type the capital "s". I think, users should be allowed to use arbitrary casing?~~

image

More information

  • User documentation of the feature available at https://docs.jabref.org/setup/customentrytypes.
  • Markdown for the documentation (to modify) at https://github.com/JabRef/user-documentation/blob/main/en/setup/customentrytypes.md
  • Fields can also have no property. That means, it is a plain text, single line text field. See also https://github.com/JabRef/jabref/blob/6269698cae437610ec79c38e6dd611eef7e88afe/src/main/java/org/jabref/model/entry/field/InternalField.java#L57

koppor avatar May 04 '23 19:05 koppor

Hey team, can i work on this issue?

eric052199 avatar May 11 '23 23:05 eric052199

As a general advice for newcomers: check out Contributing for a start. Also, guidelines for setting up a local workspace is worth having a look at.

Feel free to ask here at GitHub, if you have any issue related questions. If you have questions about how to setup your workspace use JabRef's Gitter chat. Try to open a (draft) pull-request early on, so that people can see you are working on the issue and so that they can see the direction the pull request is heading towards. This way, you will likely receive valuable feedback.

github-actions[bot] avatar May 11 '23 23:05 github-actions[bot]

Hi, can you please explain more about the first todo? Not sure about what setting zero or more FieldProperty means.

eric052199 avatar May 12 '23 05:05 eric052199

@eric052199 For me, it is not clear, what is unclear to you ^^.

I try to write a few text on "Enable setting zero ore more org.jabref.model.entry.field.FieldPropertys." Please ask which item should I refine or if you need additional information.

  1. The class /src/main/java/org/jabref/model/entry/field/FieldProperty.java#L3 models properties of a field
  2. An EntryType specifies which fields are contained
  3. Custom Entry types are stored in /src/main/java/org/jabref/model/entry/types/UnknownEntryType.java#L8
  4. Custom Fields are stored in org.jabref.model.entry.field.UnknownField
  5. From the UI, a new Custom Entry Type is generated at org.jabref.gui.preferences.customentrytypes.CustomEntryTypesTabViewModel#addNewCustomEntryType
  6. org.jabref.gui.preferences.customentrytypes.CustomEntryTypesTabViewModel#addNewField is used to add a new field to the customized entry type
  7. There, org.jabref.gui.preferences.customentrytypes.FieldViewModel class is used to put a single FieldProperty. The issue is to be able to add more than one property
  8. JabRef saves the customized entry types into the .bib file:\
    @Comment{jabref-entrytype: person: req[Name] opt[Googlescholar;Orcid]}
    
  9. an example person looks as follows:\
    @Person{demo,
      name          = {Oliver Kopp},
      googlescholar = {https://scholar.google.de/citations?user=nB2GungAAAAJ&hl=de&oi=ao},
      orcid         = {https://orcid.org/0000-0001-6962-4290},
    }
    
  10. Proposal for the new format: Use | to separate field names with the list field properties:
   .orElse(new UnknownField(fieldName));
  • Note that I added -v2, because the new format is NOT backwards-compatible with the old one and thus JabRef users might loose data if they switch versions back and forth.
  • Note that there might be no field properties given. See discussion at https://github.com/JabRef/jabref/issues/3251.
  • The proposal is NOT backwards-compatible. We can also come up with a completely new format. Nested JSON. Then, the string after jabref-entrytype-v2: needs to be forwarded to a JSON parser.

Hints:

  1. IDE hint: By holding Ctrl and then click with the mouse on an identifier, you can navigate to the usage of a class, method, etc.
  2. IDE hint: By holdingCtrl+Shift+F, you can search all text occurrences. Helpful to find some code.
  3. Please try to understand the current UI
  4. Add a sketch of the new UI - maybe this helps to understand the issue more
  5. Define the new data format for writing jabref-entrytype
  6. Add test cases for the new data format. The tests should go into org.jabref.logic.exporter.MetaDataSerializerTest.
  7. A new test class for org.jabref.logic.importer.util.MetaDataParser. Needs to be created. This can be done with Ctrl+Shift+T and then Enter. parseCustomEntryType should be tested.
  8. You can also work on org.jabref.model.entry.field.FieldFactoryTest, because org.jabref.model.entry.field.FieldFactory is called in MetaDataParser.
  9. Other tests are in org.jabref.model.entry.BibEntryTypesManagerTest
  10. More tests should go into org.jabref.logic.exporter.BibtexDatabaseWriterTest
  11. Please do test-driven development. Thus, please add test cases and then use the debugger to step into the methods. Then change the methods. With that, you save development time. If you use the GUI to test your implementation, you will be slow in developing.

koppor avatar May 12 '23 08:05 koppor

"BibTEX does not distinguish between normal and capital letters in entry and field names"

- tamethebeast p. 20

ThiloteE avatar May 15 '23 19:05 ThiloteE

The entry field casing is the least important wish of the issue. I have removed the requirement to put the focus on the important code changes.

Background information: The field capitalization refs https://github.com/JabRef/blog.jabref.org/pull/47 and https://github.com/JabRef/jabref/issues/8676

koppor avatar May 16 '23 06:05 koppor

After this is fixed, please check if https://github.com/JabRef/jabref/issues/5857 is also fixed.

koppor avatar May 21 '23 04:05 koppor

@eric052199 I saw activity on the minor TODOs of this PR. May I ask if you work on the big TODO, too?

koppor avatar Jun 08 '23 14:06 koppor

@DinjerChang @eric052199 https://github.com/JabRef/jabref/pull/9993 is now merged.

Now, it should be even more easy to work to fix this issue. Please remind my hints at https://github.com/JabRef/jabref/issues/9840#issuecomment-1545379313. Please reach out if something is unclear.

koppor avatar Jun 12 '23 18:06 koppor

@koppor Thank you very much! I'll hop on to it after finals.

DinjerChang avatar Jun 12 '23 23:06 DinjerChang

@koppor I currently used a checkComboBox from org.controlsfx.control.CheckComboBox next to the field Combox. I added all the FieldsProperty by fieldPropertyCheckComboBox.getItems().addAll(FieldProperty.values()); in CustomEntryTypesTab#initialize(). I'm wondering the timing that allow user to set the field properties. Following are my thoughts right now:

  1. After user type in the field name but before pressing the add button, enable the CheckComboBox to allow user to set field properties. Once they choose the properties, and they could press add button.
  2. After user type in the field name and add it to the field column, and then, when they select the field, enable CheckComboBox to allow user to set field properties. There will be a new "plus sign" button next to the CheckComboBox to handle the logic

Not sure if I'm on the right track and would love to discuss more

Screenshot 2023-06-18 at 5 18 12 PM

DinjerChang avatar Jun 19 '23 00:06 DinjerChang

Thank you for continuing working on this.

The dropdown with the selection allements is good.

I would suggest to modify the field labels of the add buttons to really show the action taken "Add" or "Modify".

image

The property can be empty, then you can show ("default") in gray (if that is possible in JavaFX). Otherwise, leave the box empty.

In case the user modifies the field name on the left and the field does not exist, the button label "Modify" should change ot "Add". In case the user modifies the field name to an existing field again, the button should change to "Modify" again.

You can also add a button with the small circle" next to the "Modify" button. If pressed, it will reset the field properties (and the casing of the field name) to the original value. -- In other words, a field-local version of the "Reset to default" button.

koppor avatar Jun 19 '23 09:06 koppor

Screenshot 2023-06-25 at 17 06 23

Hi Team

This is my first time in contributing to open source I have tried to do some coding on this issue and was able to attach field properties for new fields as below.

is It possible to be assigned to work on this ? I have passed the checkComboBox as a reference to obtain the values selected from it.

public void addNewField(CheckComboBox addFieldValue) { Field field = newFieldToAdd.getValue(); boolean fieldExists = displayNameExists(field.getDisplayName()); if (fieldExists) { List<FieldProperty> list = addFieldValue.getCheckModel().getCheckedItems(); field.getProperties().removeAll(field.getProperties()); for (int i = 0; i < list.size(); i++) { field.getProperties().add(list.get(i)); } }

shihar94 avatar Jun 25 '23 11:06 shihar94

@shihar94 are you also a member of @DinjerChang and @eric052199's team? I appreciate your effort, but have you seen they are already working on it?

ThiloteE avatar Jun 25 '23 11:06 ThiloteE

@ThiloteE Thank you for the immediate reply. I wasn't sure if they had completed it. Will try to move to another issue.

shihar94 avatar Jun 25 '23 11:06 shihar94

Thank you very much. In most cases, we try to keep issues and projects for students separate, so as to not interfere with any grading method that is not compatible with outside interference.

ThiloteE avatar Jun 25 '23 11:06 ThiloteE

@koppor

  • At Step 10, you mentioned: Note that is not necessary to provide properties to already known field names. It is not possible to overwrite field properties (currently). I'm wondering so when adding new field or modifying field properties, only the an UnknownField being entered or selected could have access to the the field properties drop down menu(CheckComboBox)?

  • Passing field properties in FieldFactory .orElse(new UnknownField(fieldName)); this cause a lot of issue when I modified it, I stuck on it quite few days. My solution right now is simply update the field properties in the FieldViewModel.java#toField() with field.getProperties().addAll(fieldProperties) after Field field = FieldFactory.parseField(displayName.getValue()), where Set<FieldProperty>fieldProperties will be assigned at the constructor

  • I'm wondering why we already construct a Field at CustomEntryTypesTabViewModel#addNewField and passing it to the FieldViewModel, but we reconstruct the Field again with FieldViewModel#toField(). I'm thinking can we simply assign the the passed in Field to a final variable

Correct me if I'm wrong :)

DinjerChang avatar Jun 26 '23 19:06 DinjerChang

  • At Step 10, you mentioned: Note that is not necessary to provide properties to already known field names. It is not possible to overwrite field properties (currently). I'm wondering so when adding new field or modifying field properties, only the an UnknownField being entered or selected could have access to the the field properties drop down menu(CheckComboBox)?

I don't understand "could have access". - These properties should be rendered in the UI but not modifiable for standard fields. For Unknown fields, they should both be rendered and modifable.

  • Passing field properties in FieldFactory .orElse(new UnknownField(fieldName)); this cause a lot of issue when I modified it, I stuck on it quite few days. My solution right now is simply update the field properties in the FieldViewModel.java#toField() with field.getProperties().addAll(fieldProperties) after Field field = FieldFactory.parseField(displayName.getValue()), where Set<FieldProperty>fieldProperties will be assigned at the constructor

Reads good.

  • I'm wondering why we already construct a Field at CustomEntryTypesTabViewModel#addNewField and passing it to the FieldViewModel, but we reconstruct the Field again with FieldViewModel#toField(). I'm thinking can we simply assign the the passed in Field to a final variable

Need to think longer. Can only write the rough idea: The FieldViewModel is a "factory" for the final model. The idea is that an object goes into the ViewModel, which extracts data from that. Then it is modified. Then, a new object is created from that data. Reason: the incoming object might be a "constant" object. All ViewModels should be implemented the same way.

@calixtus Maybe you have something to add?

koppor avatar Jun 29 '23 07:06 koppor

@koppor Sorry for the unclear explanation of Section 1. What I've done right now,

  • Case 1: there is a field Abstract, which is a StandardField, on the table right now. When I clicked on it, the buttonAdd/Modify is disabled.
    Screenshot 2023-06-29 at 1 01 06 PM

  • Case2: when a user select a StandardField from the field type ComboBox or type in a name of StandardField directly, let's say CitationKey, which is not on the field table yet, the system will check if the input field is not a StandardField at the CustomEntryTypesTabViewModel#addNewField by if (!fieldsForAdding.stream().anyMatch(standardField -> standardField.getDisplayName().equalsIgnoreCase(field.getDisplayName()))){ field.getProperties().addAll(fieldProperties);}. If False, we don't do anything. If True, means that it's an ** UnknownField** then we add the field properties based on what user selected. In this case, the button Add will be enabled to allow user to add the field. However, it won't set any field properties even "DATE" and "DOI" are chosen

Screenshot 2023-06-29 at 1 01 39 PM
  • Case 3: When I clicked on a UnknownField, the button Modify is enable Screenshot 2023-06-29 at 1 09 28 PM

DinjerChang avatar Jun 29 '23 20:06 DinjerChang

  • I'm wondering why we already construct a Field at CustomEntryTypesTabViewModel#addNewField and passing it to the FieldViewModel, but we reconstruct the Field again with FieldViewModel#toField(). I'm thinking can we simply assign the the passed in Field to a final variable

Need to think longer. Can only write the rough idea: The FieldViewModel is a "factory" for the final model. The idea is that an object goes into the ViewModel, which extracts data from that. Then it is modified. Then, a new object is created from that data. Reason: the incoming object might be a "constant" object. All ViewModels should be implemented the same way.

@calixtus Maybe you have something to add?

There are two places a new FieldViewModel is constructed. CustomEntryTypesTabViewModel#addNewField and EntryTypeViewModel#EntryTypeViewModel one has a Field, one only has a name. You could probably either overload the constructor or change the constructor to only accepts the name as a String. I prefer the first one: To provide a field for one constructor and overload the constructor with a String, but the toField method has to be retained.

calixtus avatar Jun 29 '23 20:06 calixtus

Case 1: This is OK. In addition, a) the field name should be shown and b) show the current field properties.

Reason for b): You rendering in "Required and optional fields" does not show the properties. This is OK, because GUI coding is hard. However, there should be a change for the average user to check the field properties. Currently, they are only visible in the code.

image

Case 2:

  • Checking if it is a standard field: Do not duplicate "semantics" of code. Please use org.jabref.model.entry.field.FieldFactory#parseField(java.lang.String): boolean isStandardField = FieldFactory.parseField(name) instanceOf StandardField;. I read from your code that you already have an instance of Field at hand. Thus, you can just use instanceof there. Note that I could not provide the full code, because fieldsForAdding is IMHO the list of all fields available in the combo box. We are talking about the place where a new entry is added.
  • The screenshot is strange. It is "CitationKey", which should also be forbidden, because it is an InternalField (add it to your thinking of case 1).
  • Let's discuss concrete examples. Then, you also have a base for test cases. Let's assume the user wants to add exampleCustomField. The user selected properties DATE and DOI. This is a strange selection. I think, you are beginning to discuss, which FieldProperties are mutually exclusive. I think, org.jabref.model.entry.field.StandardField is a good list of field property combinations. Nearly none of the fields have more than one property. Only exception FieldProperty.EXTERNAL, FieldProperty.VERBATIM for URL - and UserSpecificCommentField: FieldProperty.COMMENT, FieldProperty.MULTILINE_TEXT. --> And more thinking: FILE_EDITOR and EXTERNAL implies VERBATIM. COMMENT implies MULTILINE_TEXT. Thus, I change the specification: Allow the user to select at most one field property. If the Field is generated, AND the property is FILE_EDITOR, EXTERNAL, COMMENT, then add the implied property at the end of the viewModel, which is org.jabref.gui.preferences.customentrytypes.FieldViewModel#toField.

Case 3: Correct.

koppor avatar Jul 02 '23 22:07 koppor

@DinjerChang @eric052199 are you still on it, or can I move this back to "free to take"?

ThiloteE avatar Jul 25 '23 08:07 ThiloteE

Yes, you could move it back. Sorry for the late response :)

DinjerChang avatar Jul 28 '23 06:07 DinjerChang

Thank you for the response nevertheless :-) 👍

ThiloteE avatar Jul 28 '23 07:07 ThiloteE

Just to check, should the discussion here also consider the discussion at https://github.com/JabRef/jabref/issues/9203?

ilippert avatar Aug 10 '23 08:08 ilippert

Hi guys, the task " Enable setting zero ore more org.jabref.model.entry.field.FieldPropertys. Should be done in a combo-box allowing multiple selections." is free to take? If yes, can assigned this issue to me? I'm newcomer and i would like to work in this issue. I'm looking for projects I can contribute to.

IsaacDMz avatar Sep 05 '23 17:09 IsaacDMz

Hi @IsaacDMz, yes, I believe this issue is free to take, but from first reading of the disussion above, i wouldn't recommend this issue to newcomers to java coding. "Good first issue" means that this is a good first issue to get some insight into the JabRef codebase and to keep number of classes to be changed somewhat low - not: this is a good issue for java learners. Also the time to be invested is probably more than 2 or 3 hours work for a coder with a bit JavaFX experience.

If you think you are up to it, have fun! Feel free to use our gitter chat (https://app.gitter.im/#/room/#JabRef_jabref:gitter.im) to ask questions, if you don't understand something.

calixtus avatar Sep 05 '23 18:09 calixtus

Then let's remove this from good first issues.

ThiloteE avatar Sep 05 '23 18:09 ThiloteE

Please leave the good first issues label on. As I said, good first issues does not mean that this is a newbie issue, but a good first issue for java programmers to start in the jabref code.

calixtus avatar Sep 05 '23 18:09 calixtus

Thanks. I'm not very experienced with javaFX, I have experience with java spring for backend, but I will dedicate myself to solving this problem.

IsaacDMz avatar Sep 05 '23 19:09 IsaacDMz