Add cleanup activity for URL field by Alexandra-Stath · Pull Request #9970 · JabRef/jabref

Alexandra-Stath · 2023-06-03T23:29:57Z

Add cleanup activity for URL field. Specifically when a url is found in the note field of a BibTex entry, we place it under the URL field.

I have already made the necessary adjustments to several classes under gui and logic packages to ensure cleanup's functionality.
Added parameterized tests that validate the correctness of the process. Concidered various types of urls that need to be matched accordingly.

Mandatory checks

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Siedlerchr · 2023-06-04T06:11:25Z

+
+        String NoteFieldValue = entry.getField(NOTE_FIELD).orElse(null);
+        String urlRegex = (
+                "(?i)\\b((?:https?://|www\\d{0,3}[.]|[a-z0-9.\\-]+[.]"


Can you please add a comment/link on the regex, e.g where you got it from

This was originally fetched from stackoverflow, but there were some modifications in order for it to be functional. Shall I comment that link in my code?

Yes, and the modifications you did

Siedlerchr · 2023-06-04T06:12:37Z

 Automatically\ assign\ new\ entry\ to\ selected\ groups=Αυτόματη ανάθεση νέας καταχώρησης στις επιλεγμένες ομάδες
 %0\ mode=λειτουργία %0
 Move\ DOIs\ from\ note\ and\ URL\ field\ to\ DOI\ field\ and\ remove\ http\ prefix=Μετακίνηση του DOI (Ψηφιακό Αναγνωριστικό Αντικειμένου) από το πεδίο σημείωσης και διεύθυνσης URL, στο πεδίο DOI και αφαίρεση του προθέματος http
+Move\ URLs\ in\ note\ field\ to\ url\ field=


Please only edit the en file. All other languages are handled by crowdin

Siedlerchr · 2023-06-04T06:15:52Z

Thanks for your contribution and really great that you added tests as well!
Overall looks already good.

Siedlerchr · 2023-06-04T21:38:33Z

There is one code style check failing (open rewrite)

These recipes would make results to src/main/java/org/jabref/logic/cleanup/URLCleanup.java:
org.jabref.config.rewrite.cleanup
org.openrewrite.java.cleanup.UnnecessaryParentheses
Report available:

Run 'gradle rewriteRun' to apply the recipes.

Siedlerchr · 2023-06-04T21:42:36Z

+        // Expected entry in case note field holds more than one URLs
+        BibEntry expectedWithMultipleURLs = new BibEntry()
+                .withField(StandardField.URL, "https://hdl.handle.net/10442/hedi/6089")
+                .withField(StandardField.NOTE, "/url{http://142.42.1.1:8080}");


Can you please test with \url{} ? See also the link from koppor in the description. That would make more sense because it's a LaTeX command that is very liked to end up in the field.

koppor · 2023-06-05T10:47:40Z

+import org.jabref.model.entry.field.StandardField;
+
+/**
+ * Checks whether URL exists in note field, and stores it under url field .


No space before final dot of a sentence.

Suggested change

* Checks whether URL exists in note field, and stores it under url field .

* Checks whether URL exists in note field, and stores it under url field.

koppor · 2023-06-05T10:48:23Z

+        /**
+         * Moves URL from NOTE to URL field.
+         */


No comment necessary - the comment is in URLCleanup class - similar to the other cleanup jobs.

Suggested change

/**

* Moves URL from NOTE to URL field.

*/

koppor · 2023-06-05T10:48:48Z

 The\ output\ option\ depends\ on\ a\ valid\ input\ option.=L'option de sortie dépend d'une option d'entrée valide.
 Linked\ file\ name\ conventions=Conventions pour les noms de fichiers liés
-Filename\ format\ pattern=Modèle de format de nom de fichier 
+Filename\ format\ pattern=Modèle de format de nom de fichier


These changes will be overwritten. You need to go to Crowdin service to update issues at the translations.

koppor · 2023-06-05T10:49:22Z

+
+    private static Stream<Arguments> provideURL() {
+        /*
+         * Expected entries with various types of URLs(e.g, not secure protocol, password included for


Space before opening brace in English text

Suggested change

* Expected entries with various types of URLs(e.g, not secure protocol, password included for

* Expected entries with various types of URLs (e.g, not secure protocol, password included for

koppor · 2023-06-05T10:51:44Z

+        BibEntry[] expectedwithURL = {
+                new BibEntry().withField(StandardField.URL, "https://hdl.handle.net/10442/hedi/6089"),
+                new BibEntry().withField(StandardField.URL, "http://hdl.handle.net/10442/hedi/6089"),
+                new BibEntry().withField(StandardField.URL, "http://userid:password@example.com:8080"),
+                new BibEntry().withField(StandardField.URL, "http://142.42.1.1:8080"),
+                new BibEntry().withField(StandardField.URL, "http://☺.damowmow.com"),
+                new BibEntry().withField(StandardField.URL, "http://-.~_!$&'()*+,;=:%40:80%2f::::::@example.com"),
+                new BibEntry().withField(StandardField.URL, "https://www.example.com/foo/?bar=baz&inga=42&quux"),
+        };


These values are used only ones. Please inline them below.

Background: We aim to keep expected and actual togeher so that one can read them easily. Using this array approach, reading will get difficult. See one inline-proposal below

@koppor So I follow this approach expected/actual together, as well for the other expected entries (e.g., expectedWithTwoNoteValues and expectedWithMultipleURLs

koppor · 2023-06-05T10:52:32Z

+                Arguments.of(expectedwithURL[0], new BibEntry().withField(
+                        StandardField.NOTE, "\\url{https://hdl.handle.net/10442/hedi/6089}")),


Suggested change

Arguments.of(expectedwithURL[0], new BibEntry().withField(

StandardField.NOTE, "\\url{https://hdl.handle.net/10442/hedi/6089}")),

Arguments.of(

new BibEntry().withField(StandardField.URL, "https://hdl.handle.net/10442/hedi/6089"),

new BibEntry().withField(StandardField.NOTE, "\\url{https://hdl.handle.net/10442/hedi/6089}")),

koppor · 2023-06-05T11:02:44Z

+            String newNoteFieldValue = NoteFieldValue
+                    .replace(url, "")
+                    .replace("\\url{}", "")
+                    .replace(",", "").trim();


This won't work. in call cases. Maybe remove it - or fix it with maybe checking for regex "^, " - so that , appears at the beginning of the line AND is followed by a space.

Examples for note content:

\url{http://example.org}, cited by Kramer, 2002.

And some other strings one cannot forsee. Thus, simply removing all commas will produce wrong content.

I agree, this happens in edge cases. If you don't find a quick solution, keep as is.

@koppor, three proposed solutions here:

Either url is preceded or followed by a comma

String newNoteFieldValue = NoteFieldValue .replace(url, "") .replaceAll("(, )?\\\\?url\\{\\}(, )?", "").trim();

Not preferred but works

String newNoteFieldValue = NoteFieldValue .replace(url, "") .replace(", \\url{}", "") .replace("\\url{}, ", "") .replace("\\url{}", "").trim();

Another solution, working with the current cases

// works with: \url{http://example.org}, cited by Kramer, 2002. // doesn't work with: Cited by Kramer, 2002, \url{http://example.org} String newNoteFieldValue = NoteFieldValue .replace(url, "") .replace("\\url{}", "") .replaceFirst(", ", "").trim();

Overall, I could go with the first proposed solution as is more effective and covers plenty of cases
Also added a test case for: \url{http://example.org}, cited by Kramer, 2002.

The fist one looks good, you can also add a comment before the RegEx to explain it

…th/jabref into fix-for-koppor-216

Siedlerchr

Failing test is not related

koppor

Small nitpicks regarding casing of variable names (they should all be lower case in Java). I'll fix them for myself and then merge.

Thank you for working on this!

koppor · 2023-06-08T13:13:04Z

@Alexandra-Stath Thank you for the follow-up. I did not use any IDE and thus missed one thing. Merged now!

Alexandra-Stath · 2023-06-08T21:31:07Z

@koppor @Siedlerchr
Thank you both very much for your time and cooperation.
Wish you all the best!

koppor · 2023-10-01T14:31:12Z

+             * remove it from the note field, and no other action is performed.
+             */
+            if (entry.hasField(URL_FIELD)) {
+                String urlFieldValue = entry.getField(URL_FIELD).orElse(null);


I thought, this could lead to issues, but I did not see it. There were no tests covering the orElse branch.

In future: In case Optionals are available, make use of them!

Fix and link to issue at #10435

* 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) ...

Alexandra-Stath added 7 commits June 4, 2023 01:42

Make the necessary adjustments to several classes

a8a1251

Add cleanup activity regarding URL field

07973f7

Add parameterized tests for URLCleanup java class

e251b39

Include cleanup activity description in properties files

ef5ff9d

Update javadoc comment

00dd484

Add description in CHANGELOG.md

a586e68

Resolve merge conflicts

d572905

Siedlerchr reviewed Jun 4, 2023

View reviewed changes

Alexandra-Stath added 2 commits June 4, 2023 13:18

Update properties files

940c3ec

Update URLCleanup java class with javadoc comment about urlRegex

9c83833

Alexandra-Stath requested a review from Siedlerchr June 4, 2023 19:57

Update URLCleanup java class with javadoc comment about urlRegex

7b3878e

Siedlerchr reviewed Jun 4, 2023

View reviewed changes

Update Latex command usage and fix checkstyle errors

d69d98c

Alexandra-Stath requested a review from Siedlerchr June 4, 2023 22:36

koppor requested changes Jun 5, 2023

View reviewed changes

Alexandra-Stath and others added 6 commits June 6, 2023 20:28

Remove comment from CleanupPreferences class

7db3f62

Update cleanup activity in URLCleanup class

07db0a6

Update tests structure and add more cases

846dda5

Update comment in URLCleanup class

3b8b5db

Resolve conflicts caused in CHANGELOG.md

907d774

Merge branch 'main' into fix-for-koppor-216

e48c418

Alexandra-Stath requested a review from koppor June 6, 2023 17:52

ThiloteE added the component: cleanup-ops label Jun 6, 2023

koppor reviewed Jun 7, 2023

View reviewed changes

Comment thread src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.fxml Outdated

Alexandra-Stath added 2 commits June 7, 2023 17:04

Correction of CleanupPresetPanel and property files

583917b

Merge branch 'fix-for-koppor-216' of https://github.com/Alexandra-Sta…

dbb41af

…th/jabref into fix-for-koppor-216

Alexandra-Stath requested a review from koppor June 7, 2023 14:11

Siedlerchr previously approved these changes Jun 7, 2023

View reviewed changes

Update CHANGELOG.md

ec6ff74

koppor dismissed Siedlerchr’s stale review via ec6ff74 June 8, 2023 05:19

koppor previously approved these changes Jun 8, 2023

View reviewed changes

Apply suggestions from code review

0a0ce4f

koppor dismissed their stale review via 0a0ce4f June 8, 2023 05:26

Update of URLCleanup class

c8d45b3

koppor merged commit b4d2d51 into JabRef:main Jun 8, 2023

dkokkotas mentioned this pull request Jun 10, 2023

Add cleanup operation in reference to the Urldate field #9999

Merged

6 tasks

koppor reviewed Oct 1, 2023

View reviewed changes

	* Checks whether URL exists in note field, and stores it under url field .
	* Checks whether URL exists in note field, and stores it under url field.

	* Expected entries with various types of URLs(e.g, not secure protocol, password included for
	* Expected entries with various types of URLs (e.g, not secure protocol, password included for

		Arguments.of(expectedwithURL[0], new BibEntry().withField(
		StandardField.NOTE, "\\url{https://hdl.handle.net/10442/hedi/6089}")),

Uh oh!

Conversation

Alexandra-Stath commented Jun 3, 2023

Mandatory checks

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!

Siedlerchr commented Jun 4, 2023

Uh oh!

Siedlerchr commented Jun 4, 2023

Uh oh!

Siedlerchr Jun 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Siedlerchr left a comment

Choose a reason for hiding this comment

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koppor commented Jun 8, 2023

Uh oh!

Alexandra-Stath commented Jun 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Siedlerchr Jun 4, 2023 •

edited

Loading