Fix for issue koppor 254 (Auto cleanup url) #4255

zeabin · 2018-08-09T08:16:20Z

This issue is koppor#254.
Set auto cleanup url links on paste action and add cleanup url menu item to the url field
Most codes are modified from a previous contributor in #3394.

Currently the menu item list changed as follows(with defalut editor menu, change case, convert, etc):

But I am not sure whether it's necessary to add defalut editor menu. If not the menu item list would be:

I have run ./gradlew check but failed. But even if I switch to the master branch which is the same as JabRef/master, the build still failed with the same error message like:

But it seems that I did not modify related classes. I don't know what went wrong. It confuses me a lot.

I am looking forward to your feedback.

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

Siedlerchr · 2018-08-09T08:23:59Z

Thanks for your contribution. The gradlew check often fails locally due to some platform specific encoding issues in the terminal. You don't reed to run it as it just takes time to run all the test. We use Travis CI (Continous Integration) for testing. As long as the Travis CI test does not show any errors, it's good!)
In case Travis fails just click on it and you can see where it exactly failed.

Siedlerchr · 2018-08-09T08:27:07Z

src/main/java/org/jabref/gui/fieldeditors/EditorTextArea.java

+    /**
+     * Empty interface implementation to do nothing external on paste method
+     */
+    private static class EmptyPasteHandler implements PasteActionHandler {


You could add this simply as default method in the interface, so you won't need that emptyPastHandller I think
https://dzone.com/articles/interface-default-methods-java

Thanks for your feedback. I will try to fix this.

Since PasteActionHandler is a functional interface,

private PasteActionHandler pasteActionHandler = () -> {};

should work.

Siedlerchr

Just a minor code style improvement, but otherwise looks good!

tobiasdiez

In general the code looks good to me. I've only a few minor remarks.

tobiasdiez · 2018-08-09T09:38:27Z

src/main/java/org/jabref/gui/fieldeditors/EditorTextArea.java

+    /**
+     * Empty interface implementation to do nothing external on paste method
+     */
+    private static class EmptyPasteHandler implements PasteActionHandler {


Since PasteActionHandler is a functional interface,

private PasteActionHandler pasteActionHandler = () -> {};

should work.

tobiasdiez · 2018-08-09T10:12:15Z

src/main/java/org/jabref/gui/fieldeditors/contextmenu/EditorMenus.java

+    public static Supplier<List<MenuItem>> getCleanupURLMenu(TextArea textArea) {
+        return () -> {
+            CustomMenuItem cleanupURL = new CustomMenuItem(new Label(Localization.lang("Cleanup URL Link")));
+            //cleanupURL.setDisable(true);


remove comment or better bind the disable property to textArea.textProperty().isEmpty()

tobiasdiez · 2018-08-09T10:12:29Z

src/main/java/org/jabref/gui/fieldeditors/contextmenu/EditorMenus.java

+            List<MenuItem> menuItems = new ArrayList<>();
+            menuItems.add(cleanupURL);
+            menuItems.add(new SeparatorMenuItem());
+            menuItems.addAll(getDefaultMenu(textArea).get());


It should be the second option in your screenshots since "Protect terms" and the other options do not make sense for the URL field.

tobiasdiez · 2018-08-09T10:54:22Z

src/main/resources/l10n/JabRef_en.properties

@@ -2227,6 +2227,8 @@ Abbreviate\ journal\ names\ (ISO)=Abbreviate journal names (ISO)
 Abbreviate\ journal\ names\ (MEDLINE)=Abbreviate journal names (MEDLINE)
 Blog=Blog
 Check\ integrity=Check integrity
+Cleanup\ URL\ Link=Cleanup URL Link


it should be lower case link in both cases

tobiasdiez · 2018-08-09T10:54:22Z

src/main/resources/l10n/JabRef_en.properties

@@ -2227,6 +2227,8 @@ Abbreviate\ journal\ names\ (ISO)=Abbreviate journal names (ISO)
 Abbreviate\ journal\ names\ (MEDLINE)=Abbreviate journal names (MEDLINE)
 Blog=Blog
 Check\ integrity=Check integrity
+Cleanup\ URL\ Link=Cleanup URL Link


it should be lower case link in both cases

tobiasdiez

Thanks for the quick follow-up!

* upstream/master: (33 commits) Fix display of checkboxes Fix Remove Spin comments Fix#3861_build_addneeded/deleteobsoleteproperties2 Fix#3861_build_addneeded/deleteobsoleteproperties Fix#3861_build Fix attach file does not relativize file path (#4252) Fix for issue koppor 254 (Auto cleanup url) (#4255) Fix#ReplaceString_ChangeConflict Fix#3861ReplaceString_solveConflict Fix#3861RS_conflicttest Fix#3861RS_conflicttest Remove changelog entries Fix#3861ReplaceString_checkStyle Fix#3861ReplaceString_Improvements Fix#3861ReplaceString_ContributeLog fix log4j slf4j dependency Update dependencies (#4251) Fix author list parser (#4169) (#4248) ReplaceStringMVVMPattern ... # Conflicts: # src/main/java/org/jabref/gui/BasePanel.java

zeabin added 11 commits August 8, 2018 12:34

Add a unit test for CleanupURLFormatter

e0a849b

Add CleanupURLFormatter

8622dc8

Add Loalized value key in JabRef_en.properties

5b8f582

Add CleanupURLFormatter to method getOthers() in class Formatters.java

66b01b0

Add paste action handling in EditorTextArea

05c0909

Add cleanupURL menu item to EditorMenus

13fa7ef

Add cleanupURL menu to UrlEditor

8623675

Add auto url cleanup on paste action for UrlEditor

409641c

Update CHANGELOG.md

fa00919

Merge latest commits

9578e06

fix misspelling of localization key --Cleanup URL Link

9c35b04

Siedlerchr reviewed Aug 9, 2018

View reviewed changes

Siedlerchr approved these changes Aug 9, 2018

View reviewed changes

Fix code style problems

c6e0ec8

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 9, 2018

tobiasdiez requested changes Aug 9, 2018

View reviewed changes

zeabin added 6 commits August 9, 2018 19:19

Remove EmptyPasteHandler

60e5c77

Remove default Menus in url field

49b0202

Change word Link to link

6a93311

Set CleanupURL disable when url field is empty

c21caf9

fix missed word Link to link

7bf3997

Fix word misspelling

603e65c

tobiasdiez approved these changes Aug 9, 2018

View reviewed changes

tobiasdiez merged commit 89ee885 into JabRef:master Aug 9, 2018

zeabin deleted the fix-for-issue-koppor-254 branch August 12, 2018 10:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for issue koppor 254 (Auto cleanup url) #4255

Fix for issue koppor 254 (Auto cleanup url) #4255

zeabin commented Aug 9, 2018

Siedlerchr commented Aug 9, 2018

Siedlerchr Aug 9, 2018

zeabin Aug 9, 2018

tobiasdiez Aug 9, 2018

Siedlerchr left a comment

tobiasdiez left a comment

tobiasdiez Aug 9, 2018

tobiasdiez Aug 9, 2018

tobiasdiez Aug 9, 2018

tobiasdiez Aug 9, 2018

tobiasdiez Aug 9, 2018

tobiasdiez left a comment

Fix for issue koppor 254 (Auto cleanup url) #4255

Fix for issue koppor 254 (Auto cleanup url) #4255

Conversation

zeabin commented Aug 9, 2018

Siedlerchr commented Aug 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Siedlerchr left a comment

Choose a reason for hiding this comment

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez left a comment

Choose a reason for hiding this comment