JabRef / jabref

Graphical Java application for managing BibTeX and biblatex (.bib) databases
https://devdocs.jabref.org
MIT License
3.64k stars 2.59k forks source link

Enhance customization of entry types #9840

Open koppor opened 1 year ago

koppor commented 1 year ago

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:

image

More information

eric052199 commented 1 year ago

Hey team, can i work on this issue?

github-actions[bot] commented 1 year ago

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.

eric052199 commented 1 year ago

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

koppor commented 1 year ago

@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:
    • @Comment{jabref-entrytype-v2: person: req[Name|PERSON_NAME] opt[Googlescholar|EXTERNAL;Orcid|EXTERNAL]}
    • @Comment{jabref-entrytype-v2: customizedtype: req[title;author;date] opt[year;month;publisher]}
    • Note that is not necessary to provide properties to already known field names. It is not possible to overwrite field properties (currently). Reason: Parsing logic at /src/main/java/org/jabref/model/entry/field/FieldFactory.java#L76. You can think of changing this in your implementation. But take this as optional.
    • New field properties can be added by passing them at /src/main/java/org/jabref/model/entry/field/FieldFactory.java#L90
      .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.
ThiloteE commented 1 year ago

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

- tamethebeast p. 20

koppor commented 1 year ago

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 commented 1 year ago

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

koppor commented 1 year ago

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

koppor commented 1 year ago

@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.

DinjerChang commented 1 year ago

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

DinjerChang commented 1 year ago

@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
koppor commented 1 year ago

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.

shihar94 commented 1 year ago
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)); } }

ThiloteE commented 1 year ago

@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?

shihar94 commented 1 year ago

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

ThiloteE commented 1 year ago

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.

DinjerChang commented 1 year ago

@koppor

Correct me if I'm wrong :)

koppor commented 1 year ago
  • 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?

DinjerChang commented 1 year ago

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

Screenshot 2023-06-29 at 1 01 39 PM
calixtus commented 1 year ago
  • 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.

koppor commented 1 year ago

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:

Case 3: Correct.

ThiloteE commented 1 year ago

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

DinjerChang commented 1 year ago

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

ThiloteE commented 1 year ago

Thank you for the response nevertheless :-) 👍

ilippert commented 1 year ago

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

IsaacDMz commented 1 year ago

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.

calixtus commented 1 year ago

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.

ThiloteE commented 1 year ago

Then let's remove this from good first issues.

calixtus commented 1 year ago

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.

IsaacDMz commented 1 year ago

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 commented 1 year ago

Hey guys, I'm having a lot of difficulty completing this task, I'm going to unassign me here and try to find another task. Sorry

Siedlerchr commented 1 year ago

@IsaacDMz Okay, if you need some inspiration: You can browse through our project lists: https://github.com/orgs/JabRef/projects We tried to categorize the issues

nio2004 commented 1 year ago

Can i work on the first todo of this issue?

ThiloteE commented 1 year ago

@nio2004 sure. I will assign you.

Xavieryyu commented 1 year ago

Hi. Can I continue working on this issue?

ThiloteE commented 1 year ago

Apparently, yes. Nice catch for noticing nobody was assigned ;-)

Siedlerchr commented 10 months ago

This issue is free to take again! There is still only one thing missing

enesbkoca commented 9 months ago

Hey. I'd like to work on this issue, can you assign me to it? Thanks :)

koppor commented 8 months ago

@enesbkoca Nice! Looking forward!

enesbkoca commented 7 months ago

Hi, I've looked into this issue, and it seems to be more complicated than I initially thought and due to time constraints I'm afraid I will not be able to implement this issue. For this reason I'd like to be unassigned, my apologies.

aqurilla commented 7 months ago

Hi! I’d like to take up this issue

ThiloteE commented 7 months ago

Hi @aqurilla, nice to see you again :-)

aqurilla commented 7 months ago

@ThiloteE thankyou, happy to be back! :D

ThiloteE commented 5 months ago

This change will add to the proliferation of undocumented entry types that will probably be shared with colleagues and in the wild. Maybe there should be some kind of documentation or "info" that points to the biblatex issue tracker for people that want to make their entry type "official".

koppor commented 1 month ago

Note that "nonWrappable" fields is the current configuration for multiline fields. With "wrappable" we meant that JabRef can automatically introduce linebreaks whenever it thinks, it is good.

Siedlerchr commented 1 month ago

Originally there existed only the "wrappable fields" options, meant for saving the fields with linebreaks in between or not. This is important as otherwise, for example, a line break inside the content of the URL or DOI field would break the link in LaTeX documents when rendering. Then it was misused as an easy way for users to be able to also configure it in the UI (entry editor).

However, these two features should be decoupled now!

koppor commented 1 month ago

Originally there existed only the "wrappable fields" options, meant for saving the fields with linebreaks in between or not. This is important as otherwise, for example, a line break inside the content of the URL or DOI field would break the link in LaTeX documents when rendering. Then it was misused as an easy way for users to be able to also configure it in the UI (entry editor).

However, these two features should be decoupled now!

No - see https://github.com/JabRef/jabref/pull/12032.