Add HTML2MD conversion to abstract and comment fields #10896

koppor · 2024-02-19T20:12:20Z

@calixtus and me looked at the issue, took inspiration of student contributions and finalized the code.

@Ashwin2397 @AmandaGAnderson @paisleylewis @lucabolzonello - Thank you working for this. We took some code of you. We mentioned you as co-authors (Using Co-authored-by:). We used the email addresses you used at your commits. Are these email addresses public? Please check d0e876f and report back here or via email if it is OK or if we should use some different email address.

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.

Co-authored-by: Carl Christian Snethlage <[email protected]> Co-authored-by: Amanda Anderson <[email protected]> Co-authored-by: Paisley Lewis <[email protected]> Co-authored-by: Luca Bolzonello <[email protected]> Co-authored-by: Ashwin2397 <[email protected]>

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

Ashwin2397 · 2024-02-20T07:41:26Z

Yes @koppor that email address is valid. Thank you for putting me as a co-author, I appreciate it :).

Ashwin2397

I noticed that the test I wrote was omitted, was there anything wrong with it?

Ashwin2397 · 2024-02-20T07:48:57Z

src/main/java/org/jabref/gui/ClipBoardManager.java

+    public static boolean hasHtml() {
+        return clipboard.hasHtml();
+    }


Out of curiosity, does Java have any delegate functionality much like Ruby's forwardable?

Rub's delegate seems to be syntactic sugar, which is not available in "pure" Java.

There is Lombok's Delegate, which is kind of similar, but hard to maintain.

In general, the choice of a programming language is not only because of its syntactic features, but more about the eco system - the available libraries and tooling, community, as well as support lifecycle (and overall adaption).

Oh that's kinda cool! Is it hard to maintain because of compromised readability and navigation?

Its an additional dependency, not known widely, and it infers with the ByteCode generation. At each Java update, there could be issues making the newer JDK not working. And we want to use the latest JDK - and dont have the resources to contribute to Lombok to fix things there.

CHANGELOG.md

Ashwin2397 · 2024-02-20T07:59:20Z

src/main/java/org/jabref/gui/ClipBoardManager.java

+    public static String getHtmlContents() {
+        String result = clipboard.getHtml();
+        if (result == null) {
+            return "";
+        }
+        return result;
+    }


I initially considered refactoring getContents to take an argument to determine if it should call getString or getHtml.

The argument was going to be an enum that would default to using getString if it wasn't provided. By using the default, it wouldn't affect existing calls to getString. However, I decided against it because that level of refactoring may hurt readability and more importantly Java doesn't allow for default arguments!

There were suggestions online on how to emulate default arguments in Java, however, none of said resources gave an explanation as to why default arguments don't exist. Does anyone know why?

You could always overload a method with default arguments.

Yes, but my question is why default arguments is not supported in the method signature. I might just be overthinking it, lol.

For a why you have to ask James Gosling, Brian Goetz or other people at oracle.
I guess it was simply not a thing in 1995 and is not needed today, as you can overload methods.
It's possible that they also opted against it, since in c++ (and the syntax is very much alike) default values for method parameters are possible too since c++11 I think.

Default arguments are an indicator for multiple methods. For instance, never pass Optional as method parameter. Read more in Java by Comparison - highly recommended!

In the concrete case, passing "switches" to methods to change behavior is an indicator to use overloading. Which we did in this case. - More clear: NEVER use switches to guide behavior (e.g., if in mode html do that, if in mode XML do that, ...), use overloading.

For a why you have to ask James Gosling, Brian Goetz or other people at oracle.

I guess I'll send him a message, when he accepts my connection

Read more in Java by Comparison - highly recommended!

Thank you for the book recommendation, I'll add that to my list!

In the concrete case, passing "switches" to methods to change behavior is an indicator to use overloading. Which we did in this case. - More clear: NEVER use switches to guide behavior (e.g., if in mode html do that, if in mode XML do that, ...), use overloading.

Hmmm ... this seems to be a common pattern in other programming languages as well. Intuitively it makes sense as there would be too much logic in the giant switch method. Would you agree with this statement?

Take a look here, this is a good summary i like: https://github.com/HugoMatilla/Effective-JAVA-Summary?tab=readme-ov-file#20-prefer-class-hierarchies-to-tagged-classes
Also you could take a look of the new data oriented programming paradigm supported by the new Java language features like sealed classes and pattern matching, which aims to support programming in a similar direction.

Ashwin2397 · 2024-02-20T08:02:39Z

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

+    protected TextInputControl createTextInputControl() {
+        return isMultiLine ? new EditorTextArea() : new EditorTextField();
+    }


It looks like the instance variable isMultiLine isn't consumed anywhere else but this method 🤔 . Given this, why make it an instance variable rather than just passing it to this method?

We want to keep the API of SimpleEditor clean, since the MarkdownEditor only has Multiline fields by definition.

Ah right of course, that makes sense! Thank you!

Here you see how we implemented method overloading. The childs of this class should have their own implementation of createTextInputControl, which should not rely on the propery isMultiline. Therefore, the property was made a private variable.

calixtus · 2024-02-20T08:49:19Z

I noticed that the test I wrote was omitted, was there anything wrong with it?

The test was testing the clipboardmanager instead of the actual changed behavior of the html paste in this pr and seemed a bit trivial to us, so we did not include it.

github-actions · 2024-02-20T20:08:15Z

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

* upstream/main: (22 commits) Bump com.github.andygoossens.modernizer from 1.9.0 to 1.9.2 (JabRef#10922) Bump com.dlsc.gemsfx:gemsfx from 1.97.0 to 2.0.3 (JabRef#10923) Bump org.apache.logging.log4j:log4j-to-slf4j from 2.22.1 to 2.23.0 (JabRef#10921) Added missing changelog entry for JabRef#10912 Change the popup to enter types to the combo box for custom entry types (JabRef#10912) Add JDK EA build (JabRef#10904) Fix Broken Links (JabRef#10899) Add HTML2MD conversion to abstract and comment fields (JabRef#10896) docs: Fixed URLs and corrected typos (JabRef#10900) Add refresh button for LaTeX citations. (JabRef#10901) Farewell btut 👋 (JabRef#10905) Fix: About OOError, Alternatives section not visible (JabRef#10902) Update code-quality.md Fix documentation issues: Sourcegraph URL and method name (JabRef#10898) Removed mainapplication layer (JabRef#10895) Update check-links.yml (JabRef#10897) [WIP] Adds the ability to specify [camelN] as a title-related field marker for citation key generation (JabRef#10772) Bump com.dlsc.gemsfx:gemsfx from 1.92.0 to 1.97.0 (JabRef#10894) Bump org.openrewrite.recipe:rewrite-recipe-bom from 2.6.3 to 2.6.4 (JabRef#10892) Bump org.junit.platform:junit-platform-launcher from 1.10.1 to 1.10.2 (JabRef#10893) ... # Conflicts: # src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java

koppor marked this pull request as draft February 19, 2024 20:14

calixtus previously approved these changes Feb 19, 2024

View reviewed changes

Siedlerchr reviewed Feb 19, 2024

View reviewed changes

src/main/java/org/jabref/gui/fieldeditors/MarkdownEditor.java Outdated Show resolved Hide resolved

koppor mentioned this pull request Feb 19, 2024

10558 fix html into fields #10886

Closed

8 tasks

Add ClipBoardManager bridge

c168c9b

koppor dismissed calixtus’s stale review via c168c9b February 19, 2024 20:25

Siedlerchr approved these changes Feb 19, 2024

View reviewed changes

calixtus approved these changes Feb 19, 2024

View reviewed changes

Ashwin2397 reviewed Feb 20, 2024

View reviewed changes

Merge branch 'main' into fix-10558

fb12954

koppor marked this pull request as ready for review February 22, 2024 06:59

koppor added this pull request to the merge queue Feb 22, 2024

Merged via the queue into main with commit f570a2d Feb 22, 2024

koppor deleted the fix-10558 branch February 22, 2024 07:08

LoayGhreeb mentioned this pull request Feb 28, 2024

Fix: abstract field loses markdown formatter after resetting default preview style #10942

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add HTML2MD conversion to abstract and comment fields #10896

Add HTML2MD conversion to abstract and comment fields #10896

koppor commented Feb 19, 2024

Ashwin2397 commented Feb 20, 2024

Ashwin2397 left a comment

Ashwin2397 Feb 20, 2024

koppor Feb 21, 2024

Ashwin2397 Feb 22, 2024

koppor Feb 22, 2024

Ashwin2397 Feb 20, 2024

calixtus Feb 20, 2024

Ashwin2397 Feb 20, 2024

calixtus Feb 21, 2024

koppor Feb 21, 2024

Ashwin2397 Feb 22, 2024

Ashwin2397 Feb 22, 2024

calixtus Feb 22, 2024

Ashwin2397 Feb 20, 2024

calixtus Feb 20, 2024

Ashwin2397 Feb 20, 2024

koppor Feb 21, 2024

calixtus commented Feb 20, 2024 •

edited

Loading

github-actions bot commented Feb 20, 2024 •

edited

Loading

Add HTML2MD conversion to abstract and comment fields #10896

Add HTML2MD conversion to abstract and comment fields #10896

Conversation

koppor commented Feb 19, 2024

Mandatory checks

Ashwin2397 commented Feb 20, 2024

Ashwin2397 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

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

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

calixtus commented Feb 20, 2024 • edited Loading

github-actions bot commented Feb 20, 2024 • edited Loading

calixtus commented Feb 20, 2024 •

edited

Loading

github-actions bot commented Feb 20, 2024 •

edited

Loading