fixed the cleanup renaming which will only rename ".pdf" files. #12396

laky241 · 2025-01-18T18:22:20Z

added a new method allowedFileType which allow clean up to rename only ".pdf files"

other files like "jpeg" or "XYZ" will be skipped for renaming.

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

Siedlerchr · 2025-01-19T11:24:23Z

Please have a look at the unit tests, guess due to limiting it to pdf now tests fail and need to be adapted by adding the extension

… fix/11358-cleanup-renaming

laky241 · 2025-01-19T18:42:26Z

@Siedlerchr made the changes, how it is looking.

InAnYan · 2025-01-20T19:58:11Z

src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java

+    private boolean allowedFileType(String filePath) {
+        String lowerCase = filePath.toLowerCase();
+
+        return lowerCase.endsWith(".pdf");


Use

jabref/src/main/java/org/jabref/logic/util/io/FileUtil.java

Line 488 in 4f8ece3

public static boolean isPDFFile(Path file) {

jiucenglou · 2025-01-22T06:19:59Z

Please have a look at the the original issue which asks for more options than just renaming pdf files. For example, I asked if "-fig6" and the "-fig8" postfixes can be retained after renaming. :)

ideal behavior
old-ms-key.pdf			->	new-ms-key.pdf
old-ms-key-si.pdf		->	new-ms-key-si.pdf
old-ms-key-fig5.jpg		->	new-ms-fig5.jpg
old-ms-key-fig8.xyz		->	new-ms-fig8.xyz

Currently, the postfixes are not respected and the files are categorized by extensions and ordered in their categories.

laky241 · 2025-01-22T19:23:36Z

@jiucenglou is changes looking good or need some more work ?

Siedlerchr · 2025-01-22T19:29:00Z

src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java

-                LOGGER.info("Skipping renaming: {}", file.getLink());
+
+            String fullName = Path.of(file.getLink()).getFileName().toString();
+            int dot = fullName.lastIndexOf('.');


Check out FileUtli. there is stuff like getFileExtension
and getBaseName

jiucenglou · 2025-01-23T02:28:13Z

@jiucenglou is changes looking good or need some more work ?

Thanks ! However it is probably necessary to use regex matching instead of looking for "-" right ?

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

laky241 · 2025-01-23T19:05:40Z

Hi @Siedlerchr,
I noticed that the tests are failing due to the use of a deprecated version of actions/upload-artifact: v3 in the GitHub Actions workflow, have i made any mistake in my changes

calixtus · 2025-01-29T13:57:29Z

You can ignore the failing upload-artifact action, this is sthg we have to fix in the build system.

subhramit · 2025-02-06T22:01:24Z

src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java

@@ -32,6 +33,10 @@ public RenamePdfCleanup(boolean onlyRelativePaths, Supplier<BibDatabaseContext>
        this.filePreferences = filePreferences;
    }

+    private boolean allowedFileType(String fileName) {
+       return true;


Closing PR due to lack of quality.
@laky241 I don't mean to come off as rude, everyone has to start somewhere, but please work on basic Java and programming logic before being impatient to contribute to medium or large-scale open source projects. It wastes time for both the contributor and the maintainer, as there are way too many gaps to fill.
Have a good command over one language, build some projects, understand them end-to-end. Take your time, then come back to expand your horizon. That will go a longer way, just my two cents.

Hi @subhramit,

I appreciate your feedback and understand the importance of quality contributions. I’d like to mention that I implemented all the changes requested by @Siedlerchr and the fix has been thoroughly tested and works as expected.

However, I’m having trouble understanding what specific quality issues remain. Could you please provide more detailed guidance on which aspects of my code or approach need further improvement?

It should've been obvious by now. Take a look at the code window highlighted. What does it do?

Also, before your commit 4814a59, the snippet was different, and we know where such code comes from. We had already given you multiple flags in the first two PRs not to use AI when you don't know what you're doing.

Hi, I think what @subhramit was trying to say is: his detailed analysis of your changes lead to the assumption, that you did not see how different parts of the codebase interact with each other. In this case like changing existing tests that need very good reasoning as a test should ensure that other parts of the codebase work es expected, or how helper methods in jabref can effectively be used. @subhramit did not want to alienate you. He just thinks that starting with some issues with a smaller scope are better suited to your assumed experience with mid-to-large scale projects, to get familiarized with the whole codebase, the codestyle and modern java.
Thank you for your ongoing interest.

However, I’m having trouble understanding what specific quality issues remain.

At least, tests have to written for to reflect the issue. Edge cases need to be tested. There will be at least 10 different test cases IMHO. Single file, multipe files, with dash, with multiple dashes, dashes matching the generated filename, dashes not matching the generated filename, files following a pattern but not the configured one etc.

I sincerely apologize for all the confusion or extra work I have caused. I now understand the importance of ensuring proper test coverage and maintaining the quality standards of the project. Could you please clarify how you’d like me to proceed?

Should I continue working on this issue, or would you prefer that I move on to another task?

laky241 added 2 commits January 18, 2025 23:48

fixed the cleanup renaming which will only rename ".pdf" files.

9ad25de

Merge branch 'main' into fix/11358-cleanup-renaming

0137ba8

github-actions bot reviewed Jan 18, 2025

View reviewed changes

laky241 added 2 commits January 19, 2025 23:52

fixed the unit tests

bf0767f

Merge remote-tracking branch 'origin/fix/11358-cleanup-renaming' into…

3119815

… fix/11358-cleanup-renaming

InAnYan requested changes Jan 20, 2025

View reviewed changes

now using FileUtil.isPDFFile

3f3ddcb

laky241 requested a review from InAnYan January 21, 2025 17:22

made the requested changes

4814a59

Siedlerchr reviewed Jan 22, 2025

View reviewed changes

made the requested changes

c27c32d

github-actions bot reviewed Jan 23, 2025

View reviewed changes

Merge branch 'main' into fix/11358-cleanup-renaming

509e196

subhramit reviewed Feb 6, 2025

View reviewed changes

subhramit closed this Feb 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixed the cleanup renaming which will only rename ".pdf" files. #12396

fixed the cleanup renaming which will only rename ".pdf" files. #12396

laky241 commented Jan 18, 2025

github-actions bot left a comment

Siedlerchr commented Jan 19, 2025

laky241 commented Jan 19, 2025

InAnYan Jan 20, 2025

jiucenglou commented Jan 22, 2025

laky241 commented Jan 22, 2025

Siedlerchr Jan 22, 2025

jiucenglou commented Jan 23, 2025

github-actions bot left a comment

laky241 commented Jan 23, 2025 •

edited

Loading

calixtus commented Jan 29, 2025

subhramit Feb 6, 2025 •

edited

Loading

laky241 Feb 7, 2025

subhramit Feb 7, 2025 •

edited

Loading

calixtus Feb 7, 2025

koppor Feb 7, 2025

laky241 Feb 7, 2025

fixed the cleanup renaming which will only rename ".pdf" files. #12396

fixed the cleanup renaming which will only rename ".pdf" files. #12396

Conversation

laky241 commented Jan 18, 2025

github-actions bot left a comment

Choose a reason for hiding this comment

Siedlerchr commented Jan 19, 2025

laky241 commented Jan 19, 2025

InAnYan Jan 20, 2025

Choose a reason for hiding this comment

jiucenglou commented Jan 22, 2025

laky241 commented Jan 22, 2025

Siedlerchr Jan 22, 2025

Choose a reason for hiding this comment

jiucenglou commented Jan 23, 2025

github-actions bot left a comment

Choose a reason for hiding this comment

laky241 commented Jan 23, 2025 • edited Loading

calixtus commented Jan 29, 2025

subhramit Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

laky241 Feb 7, 2025

Choose a reason for hiding this comment

subhramit Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

calixtus Feb 7, 2025

Choose a reason for hiding this comment

koppor Feb 7, 2025

Choose a reason for hiding this comment

laky241 Feb 7, 2025

Choose a reason for hiding this comment

laky241 commented Jan 23, 2025 •

edited

Loading

subhramit Feb 6, 2025 •

edited

Loading

subhramit Feb 7, 2025 •

edited

Loading