Closed AnubhavUjjawal closed 5 years ago
@AnubhavUjjawal - Thanks for submitting this package. It is currently being reviewed.
Hi @AnubhavUjjawal, here are a few high-level comments on your package / interface:
strToArr("«ταБЬℓσ»: 1<2 & 4+1>3, foo")
work?in
argument intent for isFullString(obj)
, since you're not modifying obj
.private
so that they cannot be accessed outside of the module and will not show up in documentation.camelCase
for variables and procedure names, including module-level variables, rather than ALLCAPS
and/or under_scores
.You're welcome to make some of these changes or leave it as is. Let me know when you feel happy with your version, and I'll merge your package into the registry. Note that any updates will require you to update the v0.1.0
version tag.
@ben-albrecht Thanks for the review. I will make the updates as soon as possible, and will let you know. I agree with your points totally.
@ben-albrecht
Hi @AnubhavUjjawal, here are a few high-level comments on your package / interface:
- e.g. how should
strToArr("«ταБЬℓσ»: 1<2 & 4+1>3, foo")
work?
strToArr() returns a array with each character of string separately in array. I have made that available in the documentation. I am confused what you asked for?
- You may want an additional arg to handle how to treat characters, see string docs for more info.
I would like to leave it on default for now. Maybe later, if asked for or really needed, I would update it?
strToArr() returns a array with each character of string separately in array. I have made that available in the documentation. I am confused what you asked for?
Because string size and iterators default to bytes, unicode strings like the one above will give some unexpected results, e.g.
var s = "«ταБЬℓσ»";
writeln('size: ', s.size); // Prints 17 (bytes) instead of 8 (code points)
I would like to leave it on default for now. Maybe later, if asked for or really needed, I would update it?
That works. This was all optional feedback anyway. Thanks!
Added a StringUtils package which Closes https://github.com/chapel-lang/chapel/issues/11433