search

freshOS / Localize

🏁 Automatically clean your Localizable.strings files
MIT License
354 stars 29 forks source link

Merge similar NSLocalizedString patterns into one #13

Closed sionleroux closed 6 years ago

sionleroux commented 6 years ago

The NSLocalizedString pattern and the NSLocalizedFormatString pattern, more recently added in #12, are the same except for the name. Instead of writing them twice with different names we can write the regex once and put the differing part "Format" in an optional group:

(Format)?

This matches both, without repeating the rest of the pattern.

Disclaimer: I'm not an iOS developer, I don't have XCode and I haven't actually run this before submitting the PR, I just really like regular expressions, so please double-check this works as expected before merging 😉

sionleroux commented 6 years ago

@peter-search, maybe you ought to check this, since #12 was yours :smile:

s4cha commented 6 years ago

@sinisterstuf that looks great :) Concise is better so congrats for spotting that ! Indeed we need to test that out before merging because regexes have some syntax subtleties in iOS.

sionleroux commented 6 years ago

It should be fine because I based the escaping on the regexes nearby, but let's wait and see what actually happens on an OS X user's computer :P

On a side-note, I know it's in the code of conduct and all but I am overwhelmed by your welcoming and inclusive language. If only all pull requests were handled this way!

s4cha commented 6 years ago

@sinisterstuf, Thank YOU for the kind words and thank YOU for contributing :)

Well I have to inspect as to what it happens but it crashes in the example project provided. I'll report back when I have more details :)

ezisazis commented 6 years ago

I added a commit to this PR, so the script is not failing anymore. It was tested in a separated playground with a few example strings. You can find the code here

s4cha commented 6 years ago

@ezisazis Thank you so much for your help on this! Please excuse the late rely,

s4cha commented 6 years ago

@ezisazis Could you take a look at the remaining conflict?

sionleroux commented 6 years ago

Conflict in Localize.swift resolved by adding the whitespace pattern present in master

\\s*

used to allow arbitrary whitespace after the opening bracket. Git couldn't automatically apply the conflict resolution because the two lines modified in master are combined into one line in this branch.

s4cha commented 6 years ago

@sinisterstuf :clap: !