-
Notifications
You must be signed in to change notification settings - Fork 126
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
Uppercase keys support #9
Comments
I'd like to expand this request further. Is there a reason why phrase doesn't allow uppercase characters, symbols, or numbers? |
It seems like it ought to accept any value that is valid as an Android resource. I'm willing to do a PR on this but I'm curious if there is a reason it was purposefully avoided before I jump in. |
@dlew ditto. I have something in the works but I've been waiting for input on this issue. Their exists a test case stating no uppercase characters so I'd like some feedback on why. |
If that's an expected behaviour I suggest to include this in the documentation. I struggled quiet a while to track down the cause of error |
We don't support uppercase characters because we haven't seen a need. If anything we could ignore case, but why? Closing, feel free to reopen if you're still interested and care to make the case. Sorry for the pun. |
This issue is a key one. Is what Phrase newcomers expects Phrase to do. Please, reopen it and accept #17. Thanks! |
Just because you don't see a need doesn't mean there isn't one. I have a use case of migrating from a different substitution library that used uppercase for their keys and I can't have all my users manually translating their uppercase parameters to lowercase. I've made the case. The bigger question is instead of "why", is "why not"? |
Nope. For libraries the question is always "why". |
I think there's a very good case for fixing the terrible error message. When that's done I think this will matter less to folks as they'll get immediate feedback on the problem, and won't have to dig. I'll see if I can put a pull request together, I just ran into this last week. |
Oh, #33 almost does this, test is a little weak though. |
Ok, then I'll rephrase my question, "why lowercase"? Someone made a binary decision... could have been uppercase. |
It's what Android uses for names. It's what JSON used for keys. It's an
obvious choice over uppercase.
…On Wed, Jul 19, 2017, 6:03 PM Ron Yust ***@***.***> wrote:
Ok, then I'll rephrase my question, "why lowercase"? Someone made a binary
decision... could have been uppercase.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#9 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEER6AvvclDS8F7hoyqZcQCvnG-lemks5sPn0bgaJpZM4DUojK>
.
|
The Android standard for constants seems to be uppercase in most of the projects I've seen. Parameterized strings with uppercase keys would also be much easier to identify... so in another time and place the uppercase decision could have as easily been justified by someone else. Not trying to give you a hard time, and actually I don't care as I've moved on to different software, but the polarized right/wrong views just irk me. |
That's Java constants, not Android resources. No one said uppercase is
wrong, it's merely not something supported by design. A reduced feature set
focuses a library. One which says "yes" to everything isn't a library I
want to use, or one we want to build.
…On Wed, Jul 19, 2017, 7:32 PM Ron Yust ***@***.***> wrote:
The Android standard for constants seems to be uppercase in most of the
projects I've seen. Parameterized strings with uppercase keys would also be
much easier to identify... so in another time and place the uppercase
decision could have as easily been justified by someone else.
Not trying to give you a hard time, and actually I don't care as I've
moved on to different software, but the polarized right/wrong views just
irk me.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#9 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEelTgGMwtQbAUOHstIklYtNvrq9aks5sPpICgaJpZM4DUojK>
.
|
If it doesn't matter to you, then maybe you shouldn't be taking up other peoples' time and effort with the matter? Maintaining a library is a chore, there's no need to make it harder than it already is. |
At least, improve the error message What about |
See #35 for a fix to the error messages for invalid key names. |
Hi,
I'm getting a crash when using Phrase.from("This is an {EXAMPLE}");
Throwing a 'Unexpected character 'E'; expected key.'
Could you add support for uppercase keys? It would help me since i'm not controlling the strings I receive.
Thank you !
The text was updated successfully, but these errors were encountered: