Skip to content

Conversation

@CHB-0r1s
Copy link
Contributor

After research of all possible combinations, there is such schema for prevent useless mutations:

Without mutations

  1. split() -> split()
  2. rsplit() -> rsplit()
  3. split(" ") -> split(" ")
  4. rsplit(" ") -> rsplit(" ")
  5. split(sep=" ") -> split(sep=" ")
  6. rsplit(sep=" ") -> rsplit(sep=" ")
  7. split(maxsplit=-1) -> split(maxsplit=-1)
  8. rsplit(maxsplit=-1) -> rsplit(maxsplit=-1)
  9. split(" ", maxsplit=-1) -> split(" ", maxsplit=-1)
  10. rsplit(" ", maxsplit=-1) -> rsplit(" ", maxsplit=-1)

With mutations

  1. split(maxsplit=1) -> rsplit(maxsplit=1)
  2. rsplit(maxsplit=1) -> split(maxsplit=1)
  3. split(" ", 1) -> rsplit(" ", 1)
  4. rsplit(" ", 1) -> split(" ", 1)
  5. split(" ", maxsplit=1) -> rsplit(" ", maxsplit=1)
  6. rsplit(" ", maxsplit=1) -> split(" ", maxsplit=1)

I save it as a note in comments.

@CHB-0r1s
Copy link
Contributor Author

I hope this fix can help with #389

@CHB-0r1s
Copy link
Contributor Author

@Otto-AA @boxed This magic with parsing -1 and +1 in libcst parser cause a lot of troubles for me. So, please, if you know better way to do it, I appreciate your help.

@Otto-AA
Copy link
Collaborator

Otto-AA commented Jun 15, 2025

Hi, thank you for this PR!

@Otto-AA @boxed This magic with parsing -1 and +1 in libcst parser cause a lot of troubles for me. So, please, if you know better way to do it, I appreciate your help.

I don't think that x.split(maxsplit=-1) is written directly in the code. This is equivalent to x.split(), which I would expect people to use, rather than explicitly providing and disabling maxsplit.

So I would suggest that we do not check if the value is -1 and simply always mutate, if maxsplit is provided with any value. So x.split(' ', -1) would also be mutated to x.rsplit(' ', -1), which is not optimal but should happen very seldomly.

@CHB-0r1s
Copy link
Contributor Author

Thank you @Otto-AA !
I will apply your suggestions as soon as possible today)

@CHB-0r1s
Copy link
Contributor Author

So, new schema

Without mutations

  1. split() -> split()
  2. rsplit() -> rsplit()
  3. split(" ") -> split(" ")
  4. rsplit(" ") -> rsplit(" ")
  5. split(sep=" ") -> split(sep=" ")
  6. rsplit(sep=" ") -> rsplit(sep=" ")

With mutations

  1. split(" ", 1) -> rsplit(" ", 1)
  2. rsplit(" ", 1) -> split(" ", 1)
  3. split(maxsplit=1) -> rsplit(maxsplit=1)
  4. rsplit(maxsplit=1) -> split(maxsplit=1)
  5. split(maxsplit=-1) -> rsplit(maxsplit=-1)
  6. rsplit(maxsplit=-1) -> split(maxsplit=-1)
  7. split(" ", maxsplit=-1) -> rsplit(" ", maxsplit=-1)
  8. rsplit(" ", maxsplit=-1) -> split(" ", maxsplit=-1)
  9. split(" ", maxsplit=1) -> rsplit(" ", maxsplit=1)
  10. rsplit(" ", maxsplit=1) -> split(" ", maxsplit=1)

@CHB-0r1s
Copy link
Contributor Author

Hello! I applied all suggestions. Code looks pretty easy now. Schema was removed from comments (add link).
Can you check new updates, please :3

@Otto-AA @boxed

@CHB-0r1s
Copy link
Contributor Author

Just little ping @Otto-AA

@Otto-AA Otto-AA merged commit 5688c67 into boxed:main Jun 20, 2025
5 checks passed
@Otto-AA
Copy link
Collaborator

Otto-AA commented Jun 20, 2025

Thanks for the updates, looks good!

And no need to ping, whenever I have time I will go through all unresolved conversations. It can also take a week or two if I'm busy with life, but I won't forget you when I find the time :)

@Otto-AA Otto-AA mentioned this pull request Jun 20, 2025
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.

2 participants