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.

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

Enhance issue #5507 Add switch button and pages layout for fetchers/websearch #10595

Closed

Conversation

JunyuLuo123
Copy link

@JunyuLuo123 JunyuLuo123 commented Oct 28, 2023

Enhances #5507

Add the switch button and pages layout.

* improve medline

* checksstyle

* move

* fix checkstyle

---------

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

image
While the page is in 1. The previous button will not be access.

@ThiloteE ThiloteE changed the title Enhance issue #5507 Enhance issue #5507 (Paging support for fetchers/websearch) - Add switch button and pages layout Oct 28, 2023
@ThiloteE ThiloteE changed the title Enhance issue #5507 (Paging support for fetchers/websearch) - Add switch button and pages layout Enhance issue #5507 Add switch button and pages layout for fetchers/websearch Oct 28, 2023
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

The PR is far from complete. The connection to the fetchers is not there. This is more complicated than just adding a button somewhere in the dialog

Comment on lines +58 to +68
private final XMLInputFactory xmlInputFactory;

public MedlineImporter() {
this.xmlInputFactory = XMLInputFactory.newInstance();
// prevent xxe (https://rules.sonarsource.com/java/RSPEC-2755)
xmlInputFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
// required for reading Unicode characters such as &#xf6;
xmlInputFactory.setProperty(XMLInputFactory.IS_COALESCING, true);
// TODO: decide if necessary, if disabled MedlineImporterTestNbib fails
xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, true);
}
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Author

@JunyuLuo123 JunyuLuo123 Oct 29, 2023

Choose a reason for hiding this comment

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

Sorry I got no idea, I didn't modify this part.

Comment on lines +9 to +11
Previous=Create property
Next=Create property
Page\:=Create property
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this? -- Did you modify the wrong file? JabRef_en.properties. See https://devdocs.jabref.org/code-howtos/localization.html for more information

Copy link
Author

Choose a reason for hiding this comment

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

When I create a button name and page layout it will show error if I didn't do that.

@@ -49,13 +50,16 @@
import jakarta.inject.Inject;
import org.controlsfx.control.CheckListView;

@SuppressWarnings("checkstyle:RegexpMultiline")
Copy link
Member

Choose a reason for hiding this comment

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

Why this?

Copy link
Author

Choose a reason for hiding this comment

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

It changing automaticity. I didn't do anything on that.

@koppor koppor mentioned this pull request Oct 28, 2023
6 tasks
@JunyuLuo123
Copy link
Author

Sorry about that. Cause the time limit that's what I can think and done.

@calixtus
Copy link
Member

calixtus commented Nov 9, 2023

Hi @JunyuLuo123 , what's the status here? Are you planning to continue this PR?

@calixtus calixtus marked this pull request as draft November 9, 2023 19:28
@calixtus
Copy link
Member

As the user obviously lost interest in this PR, i'm closing it for housekeeping. Feel free to open a new PR if you decide to continue your work.

@calixtus calixtus closed this Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants