Skip to content

Fixed Issue 13418: "Search ShortSience" should do a latex-to-unicode conversion#13532

Merged
koppor merged 6 commits into
JabRef:mainfrom
xIshanSandhux:fix-for-issue-13418
Jul 15, 2025
Merged

Fixed Issue 13418: "Search ShortSience" should do a latex-to-unicode conversion#13532
koppor merged 6 commits into
JabRef:mainfrom
xIshanSandhux:fix-for-issue-13418

Conversation

@xIshanSandhux

@xIshanSandhux xIshanSandhux commented Jul 12, 2025

Copy link
Copy Markdown
Contributor

Closes #13418

Fix Description

Basically the title was not being filtered before the URL for "Search ShortSience" was built. so I updated it to passing the filtered title. I am filtering the title using org.jabref.model.strings.LatexToUnicodeAdapter.
Specifically i am using the format method as it has a fallback in it to return the original text if no latex is found, so I did not have to use any if statements or the .orElse()

Steps to test

I have created the test getShortScienceSearchURLWithoutLaTeX() in ExternalLinkCreator.java

The test is basically doing the following:

  1. Creating a new BibEntry
  2. Setting the title which contains latex in it
  3. created 2 variables (url and expectedUrl)
  4. url calls the getShortScienceSearchURL() function which returns the url constructed
  5. expectedUrl is basically just a string which contains the url which is required.
  6. Then i just do assertEquals to check if the url and expectedUrl is the same or not.

Manual Testing (if anyone wants to)

  1. add a new entry. I created a new one using the biblatex source and used the following which was provided in the issue:
    @Article{Kopp2009,
    author = {Oliver Kopp and Daniel Martin and Daniel Wutke and Frank Leymann},
    journal = {Enterprise Modelling and Information Systems},
    title = {{The Difference Between Graph-Based and Block-Structured Business Process Modelling Languages}},
    year = {2009},
    month = jun,
    number = {1},
    pages = {3--13},
    volume = {4},
    }
  2. Save it
  3. The click "search ShortScience" from the options. I just did right click on the entry.
image
  1. It then leads you to the ShortScience page and the title entered is without latex
image

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (if change is visible to the user)
  • 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

Siedlerchr commented Jul 12, 2025 via email

Copy link
Copy Markdown
Member

@xIshanSandhux

xIshanSandhux commented Jul 12, 2025

Copy link
Copy Markdown
Contributor Author

@Siedlerchr thank you for the confirmation that my code is in the right direction.

But yeah, i still need to add the test case, which is why i just created a draft PR which only has the fix in it.
I have manually tested it and its working fine.

I will have the PR in "Ready for review" once I am done with the test case and the required format for submitting the PR!

private final StateManager stateManager;
private final GuiPreferences preferences;


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please do not add additoinal empty lines.

Maybe, the changes of this class can be reverted as only empty lines were added?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh yeah for sure ill remove them, its just i added a statement there and then deleted it, hence the empty line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@calixtus

Copy link
Copy Markdown
Member

Please create a proper title for your pr. We don't have a magic fortune teller ball.
"Fixed the issue" is improper.
The title of the pr will show as the commit message when merged. Laziness has no place in open source and in a development team.

@xIshanSandhux

Copy link
Copy Markdown
Contributor Author

@calixtus yes you are right. but as I said in the earlier comments its a draft PR because its still a work in progress. I will have everything pitch perfect when i change it to "Ready for review". but i will keep in mind for future pull requests even if they are draft to name them better.

@xIshanSandhux xIshanSandhux changed the title fixed the issue Fixed Issue 13418: "Search ShortSience" should do a latex-to-unicode conversion Jul 13, 2025
Comment thread jablib/src/test/java/org/jabref/logic/util/ExternalLinkCreatorTest.java Outdated
Comment thread jablib/src/main/java/org/jabref/logic/util/ExternalLinkCreator.java
@xIshanSandhux xIshanSandhux marked this pull request as ready for review July 13, 2025 23:09
@xIshanSandhux xIshanSandhux requested a review from koppor July 13, 2025 23:09
@trag-bot

trag-bot Bot commented Jul 13, 2025

Copy link
Copy Markdown

@trag-bot didn't find any issues in the code! ✅✨

@xIshanSandhux xIshanSandhux left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well did a final review of my changes, i think they seem good to go. but lmk if i need to change/update anything.

@xIshanSandhux

Copy link
Copy Markdown
Contributor Author

Hey @koppor !

just wondering if you can review my PR and lmk if i need to make any changes or am i good to merge.

Thanks !

@koppor koppor added this pull request to the merge queue Jul 15, 2025
Merged via the queue into JabRef:main with commit abb2862 Jul 15, 2025
1 check passed
Siedlerchr added a commit that referenced this pull request Aug 2, 2025
* upstream/main:
  Bump com.squareup.okhttp3:okhttp from 5.0.0 to 5.1.0 in /versions (#13541)
  Fixed Issue 13418: "Search ShortSience" should do a latex-to-unicode conversion (#13532)
  New Crowdin updates (#13548)
  Add focus to field Link in the "Add file link" dialog. (#13520)
  Bump org.ow2.asm:asm from 9.6 to 9.8 in /versions (#13547)
  Make validation optional for saving library properties preferences (#13488)
  Bump org.apache.commons:commons-lang3 from 3.17.0 to 3.18.0 in /versions (#13546)
  --porcelain does not output any info or warn errors on the console (#13469)
  Bump jablib/src/main/resources/csl-locales from `7e137db` to `3bad433` (#13545)
  Refine "File exists" warning (#13491)
  Bump org.openrewrite.recipe:rewrite-recipe-bom from 3.8.0 to 3.11.1 (#13544)
  Bump org.openrewrite.rewrite from 7.8.0 to 7.11.0 (#13542)
  Bump jablib/src/main/resources/csl-styles from `704ff9f` to `59f124d` (#13540)
  Add architecture test for logging infrastructure (#13534)
  More harsh test on title of PR
  New Crowdin updates (#13533)
  Update extra-java-module-info plugin (#13527)
  [Junie]: fix: display help output for --help argument (#13446)
  Refactor OpenExternalFileAction  (#13508)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Search ShortSience" should to a latex-to-unicode conversion

4 participants