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.

Sign up for GitHub

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

Jump to bottom

Quick import should support RFC links #11949

Closed
koppor opened this issue Oct 13, 2024 · 8 comments · Fixed by #12043
Closed

Quick import should support RFC links #11949

koppor opened this issue Oct 13, 2024 · 8 comments · Fixed by #12043
Assignees
@AU0430
Labels
FirstTimeCodeContribution Triggers GitHub Greeter Workflow good first issue An issue intended for project-newcomers. Varies in difficulty.

Comments

@koppor
Copy link
Member

koppor commented Oct 13, 2024

I want to quickly import RFC 7276 using JabRef.

The URL is https://www.rfc-editor.org/rfc/rfc7276.html#section-2.2.2.

I would like to use the quick import

Image

It says, however, that no Id was found.

Pasting it into the main table leads to

Image

org.jabref.logic.importer.ImportException: Could not find a suitable import format.
at [email protected]/org.jabref.logic.importer.ImportFormatReader.importUnknownFormat(Unknown Source)

Using the Id fetcher explicitly, works

Image
@koppor koppor added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Oct 13, 2024
@github-project-automation github-project-automation bot moved this to Free to take in Good First Issues Oct 13, 2024
@AU0430
Copy link
Contributor

AU0430 commented Oct 13, 2024

Hello, I have read and understand the entire issue and am very interested in this issue. Can it be assigned to me?

@Siedlerchr Siedlerchr moved this from Free to take to Assigned in Good First Issues Oct 13, 2024
@koppor koppor added the FirstTimeCodeContribution Triggers GitHub Greeter Workflow label Oct 14, 2024
@github-actions GitHub Actions
Copy link
Contributor

Welcome to the vibrant world of open-source development with JabRef!

Newcomers, we're excited to have you on board. Start by exploring our Contributing guidelines, and don't forget to check out our workspace setup guidelines to get started smoothly.

In case you encounter failing tests during development, please check our developer FAQs!

Having any questions or issues? Feel free to ask here on GitHub. Need help setting up your local workspace? Join the conversation on JabRef's Gitter chat. And don't hesitate to open a (draft) pull request early on to show the direction it is heading towards. This way, you will receive valuable feedback.

Happy coding! 🚀

@AU0430
Copy link
Contributor

AU0430 commented Oct 17, 2024 •
edited
Loading

Hello, In this issue, I have successfully reproduced the RFC link import error in JabRef and observed that it can be found and imported correctly with the DOI format. This means that the problem may be in the parsing and importing function of the RFC link, rather than the overall problem of the ID import function. We can try the following two solutions:

  1. URL Correction: JabRef's quick import may not parse RFC links correctly. But I found that DOI format links can be recognized in 10.17487/RFC7276.Image
    Therefore, a potential solution is to add special processing logic for RFC urls in the code.

  2. Jabref supports RFC-specific parsers: JabRef currently supports successfully importing RFCs by explicitly selecting ID fetchers. If we can automatically detect the RFC format during the quick import process and call the RFC fetcher, the problem may be solved. I think the solution can be to detect whether it contains the word "rfc" when parsing the URL, and automatically select the RFC fetcher for processing. (For this solution, I can add a preprocessing module for RFC links: Add a preprocessing module to the quick import function to check whether the URL link conforms to the RFC link pattern. If it is an RFC link, it will be converted into a standard ID format that can be used for RFC fetcher, such as rfc7276, so that users do not need to do it manually.)

@AU0430
Copy link
Contributor

AU0430 commented Oct 17, 2024

My suggestion is the second option, because there is a ready-made RFC fetch, and the content of the doi method is slightly different from the content generated by RFC fetch.Image

@koppor
Copy link
Member Author

koppor commented Oct 17, 2024

My suggestion is the second option, because there is a ready-made RFC fetch, and the content of the doi method is slightly different from the content generated by RFC fetch.Image

Please adapt org.jabref.logic.importer.CompositeIdFetcher#performSearchById accordingly.

Add a new class RfcId similar to org.jabref.model.entry.identifier.ISBN#parse. Use this class in ´performSearchById. If valid RfcId, use org.jabref.logic.importer.fetcher.RfcFetcher`

@AU0430
Copy link
Contributor

AU0430 commented Oct 17, 2024

Thanks for the guidance! I'm going to follow your advice and handle the RFC ID by detecting it and calling RfcFetcher . I will update the question once the changes are ready!

@AU0430 AU0430 mentioned this issue Oct 20, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 23, 2024
@AU0430 @ChengPuPu @koppor
Support RFC links in Quick Import functionality (#11949) (#12043)
b90ef75 
* Add RfcId to handle RFC url link to support quick import

* Refactor RFC URL parsing
Add JavaDoc comments
Remove unnecessary lowercase

* Refactor RFC URL parsing
Add JavaDoc comments
Remove unnecessary lowercase

* Adjust code style

* Adjust code style

* According to CI test feedback:
- Removed RFC entry from identifier list(StandardField.java)
- Modified RfcId class to extend EprintIdentifier

* Accorfing to maintainer feedback to fix:
- Replaced RfcId name with RFC class for consistency and clarity.
- Updated RFC parsing logic to support both plain RFC IDs (e.g., "rfc7276") and full URLs (e.g., "https://www.rfc-editor.org/rfc/rfc7276.html").
- Added tests in RFCTest to validate proper parsing of RFC identifiers from both formats.
- Removed unnecessary method (isValid) improved code clarity.
- Adjusted the CompositeIdFetcher to integrate the new RFC class functionalities.

* Refactored RFCTest to adhere to the suggestions provided:
- Converted `testParsePlainRfcId` to a `@ParameterizedTest` using `@CsvSource`.
- Adjusted `testInvalidRfc` to use `assertEquals(Optional.empty(), rfc)`.
- Updated `testGetExternalUri` to verify the URI using `Optional.of()` for consistent checks.

* adjust code format according to CI test(checkstyle)

* Swap expected and input

* Fix OpenRewrite issues

* fix code style according to checkstyle

---------

Co-authored-by: Cheng <[email protected]>
Co-authored-by: Oliver Kopp <[email protected]>
@kushagradwi
Copy link

Is this issue still open?

@ThiloteE
Copy link
Member

ThiloteE commented Jan 6, 2025

As far as I am aware, the PR addresses all functionality requested, so let's close.

@ThiloteE ThiloteE closed this as completed Jan 6, 2025
@github-project-automation github-project-automation bot moved this from Assigned to Done in Good First Issues Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AU0430 AU0430

Labels
FirstTimeCodeContribution Triggers GitHub Greeter Workflow good first issue An issue intended for project-newcomers. Varies in difficulty.
Projects
Good First Issues
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support RFC links in Quick Import functionality (#11949) AU0430/jabref
4 participants
@koppor @kushagradwi @ThiloteE @AU0430