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

Implement filename renaming library #3913

Merged
merged 13 commits into from
Sep 18, 2023

Conversation

TranceLove
Copy link
Collaborator

Description

It is a port of https://github.com/jonschlinkert/add-filename-increment for node.js into Kotlin, with some tweaks and tricks specific to Kotlin.

Issue tracker

Addresses #2078

Automatic tests

  • Added test cases

Manual tests

  • Done

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembledebug
  • ./gradlew spotlessCheck

Related PR

Related to PR #3886

It is a port of https://github.com/jonschlinkert/add-filename-increment for node.js into Kotlin, with some tweaks and tricks specific to Kotlin.

Addresses TeamAmaze#2078
@TranceLove TranceLove marked this pull request as ready for review August 19, 2023 08:02
@TranceLove TranceLove mentioned this pull request Aug 23, 2023
4 tasks
@yatiksihag01
Copy link
Contributor

Hi @TranceLove, I've noticed a couple of issues in your code:

  • When renaming directories, the increment() function currently appends a dot (.) at the end of the directory name. It should append numbers like (1), (2), and so on. Please check line 155 in FilenameHelper.kt.
  • If I pass value of startArg 0 or 1 (default), it starts from appending (2), (3) and so on. However, when I use a higher value like 5, it works correctly.

VishnuSanal and others added 9 commits September 7, 2023 23:13
Signed-off-by: VishnuSanal <t.v.s10123@gmail.com>
Signed-off-by: VishnuSanal <t.v.s10123@gmail.com>
Signed-off-by: VishnuSanal <t.v.s10123@gmail.com>
Signed-off-by: VishnuSanal <t.v.s10123@gmail.com>
Signed-off-by: VishnuSanal <t.v.s10123@gmail.com>
Signed-off-by: VishnuSanal <t.v.s10123@gmail.com>
Check and concatenate the extension (string that follows the final dot) only if it's not empty
@TranceLove
Copy link
Collaborator Author

TranceLove commented Sep 7, 2023

@yatiksihag01 thanks for the feedback. Added c4ad60f to address the first issue, please take a look.

But for the second problem... may you be more specific about the symptom(s) you have? Say when you call increment() with the default startArg, what should I expect and why setting startArg to high values would work?

}
// Windows and default formatting are the same.
else -> {
if (n > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the second problem. Let's say startArg initial value is 1. When increment() function initially call format() , it'll simply return the same filename because n <= 1. Then increment() will call format() for n = 2 and it'll work fine. I don't know if there's specific reason to add this if (n>1) condition but in my case this is creating the problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's understandable, if looking at the unit test for windows.

fun testWindowsIncrementSimple() {
val pairs = arrayOf(
Pair("/test/file.txt", "/test/file (2).txt"),
Pair("sub/foo.txt", "sub/foo (2).txt"),
Pair("sub/nested/foo.txt", "sub/nested/foo (2).txt"),
Pair("/test/afile", "/test/afile (2)")
)
performTest(pairs, true)
}

Couldn't comment if it's right or wrong, as I was doing a direct port... but if I understand correctly, looks like you'd want to have filenames from starting with file (2).txt to file (1).txt, which is different from the original node.js implementation.

If that is the case, see if 1f2cd85 fits your needs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, now it's working fine.

Make increment starts from 1. So that in Windows/Default formatting scheme, the first incremented will begin with filenames like "file (1).txt" instead of "file (2).txt".

Note: this behaviour is different from original node.js library
@VishalNehra VishalNehra merged commit 6f36f4b into TeamAmaze:release/4.0 Sep 18, 2023
@VishalNehra VishalNehra added this to the v4.0 milestone Sep 18, 2023
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.

4 participants