Skip to content
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

User specific comment #9727

Merged
merged 37 commits into from
Jun 6, 2023
Merged

User specific comment #9727

merged 37 commits into from
Jun 6, 2023

Conversation

NiD133
Copy link
Contributor

@NiD133 NiD133 commented Apr 2, 2023

Fixes #543
1
2
3

Compulsory checks

@koppor
Copy link
Member

koppor commented Apr 4, 2023

If possible, the new editor should also support editing org.jabref.model.entry.BibEntry#commentsBeforeEntry

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good start 👍

Test cases are missing.


public class UserSpecificCommentField implements Field {
private final String name;
private final Set<FieldProperty> properties;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be static final

Set<Field> otherFields = entry.getFields().stream().filter(field -> !allKnownFields.contains(field)).collect(Collectors.toCollection(LinkedHashSet::new));

Set<Field> otherFields = entry.getFields().stream()
.filter(field -> !allKnownFields.contains(field) && !field.getName().toLowerCase().contains("comment"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be startsWith and comment-, because UserSpecificCommentField.java is implemented like that.

Maybe also check for plain comment in addition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the following not work here?

(that code is from above)

   field -> field.equals(StandardField.COMMENT) || field instanceof UserSpecificCommentField

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/entryeditor/CommentsTab.java Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@Siedlerchr
Copy link
Member

Please have a look at the failing unit tests, most of them are related to the l10n https://devdocs.jabref.org/code-howtos/localization.html

@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label May 6, 2023
@NiD133
Copy link
Contributor Author

NiD133 commented May 20, 2023

Please have a look at the failing unit tests, most of them are related to the l10n https://devdocs.jabref.org/code-howtos/localization.html
Got it! Working on it right now.

@Siedlerchr
Copy link
Member

Please have a look at the failing tests!

@Siedlerchr
Copy link
Member

Thanks for the continuing work here! What is the current status`Is this ready from your side? Anything left or unclear?

@NiD133
Copy link
Contributor Author

NiD133 commented Jun 4, 2023

Thanks for the continuing work here! What is the current status`Is this ready from your side? Anything left or unclear?

Yeah, I believe it's ready from my side. If there are any further changes or clarifications needed, please let me know. :)

@koppor koppor force-pushed the user_specific_comment branch 2 times, most recently from 30e01c4 to 0d7da65 Compare June 6, 2023 05:26
@koppor
Copy link
Member

koppor commented Jun 6, 2023

I made updates:

  • Merge "All Comments" into "Comments" tab. Thereby modifying the logic of the entry editor to prioritize the code over user-configuration 😇
  • Add more tests

@koppor
Copy link
Member

koppor commented Jun 6, 2023

In principle, this works. If one modifies the BibTeX source code, one has to switch to another entry and back to get the change updated in the Comments tab. I did not see an easy solution for this. Thus, it is good to go from my side!

@koppor koppor added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers status: changes required Pull requests that are not yet complete and removed status: changes required Pull requests that are not yet complete labels Jun 6, 2023
@koppor koppor marked this pull request as ready for review June 6, 2023 06:09
@koppor koppor removed the status: changes required Pull requests that are not yet complete label Jun 6, 2023
@NiD133
Copy link
Contributor Author

NiD133 commented Jun 6, 2023

I made updates:

  • Merge "All Comments" into "Comments" tab. Thereby modifying the logic of the entry editor to prioritize the code over user-configuration 😇
  • Add more tests

Got it!

@Siedlerchr
Copy link
Member

Thanks a lot for this cool feature!

@Siedlerchr Siedlerchr merged commit 76e2ce7 into JabRef:main Jun 6, 2023
@koppor koppor mentioned this pull request Jun 14, 2023
6 tasks
@koppor
Copy link
Member

koppor commented Oct 31, 2023

What we oversaw during review: Ordering of fields.

First, the bibtex standard field should come, then the user-specific comment fields. Since they are mixed, users are very confused and angry (see #10424). - I don't remember any other occasion where we have three thumbs ups ^^

I tried to fix it at #10610.

Just as information to @NiD133 if you work on UX in the future in other projects.

Siedlerchr added a commit that referenced this pull request Jan 1, 2024
* upstream/main: (68 commits)
  Fix issue 9863 - Change Tab selection order (#9907)
  New Crowdin updates (#9994)
  Enable editing typo with double clicking on field name in custom entry type (#9977)
  Remove "Field name:" from localization - we already have "Field name" (#9991)
  Changed database to catalog in org.jabref.gui.slr and org.jabref.logic.crawler (#9989)
  Use environment variables for hostname detection (#9910)
  Add cleanup activity for URL field (#9970)
  Fix freezing when fetching IBSN and no results are found (#9987)
  Make Group(Node)TreeViewModel more OO (#9978)
  Fix container for group item count still visible if display count is off  (#9980)
  Fix paste of BibTeX data (#9985)
  Bring back SimplifyBoolean* and UnnecessaryParantheses (and refine guide) (#9981)
  Add new menu entry to remove braces from selection, aka unprotect it. (#9968)
  User specific comment (#9727)
  Bump org.openrewrite.recipe:rewrite-recipe-bom from 1.19.3 to 2.0.0 (#9975)
  Fix typos
  Remove non-needed link
  Fix typos
  Enable RemoveJavaDocAuthorTag (#9973)
  Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9974)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
entry-editor status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add user-specific comment field
4 participants