Closed GoogleCodeExporter closed 9 years ago
[deleted comment]
Here is a try for a patch to implement the new keywords. Did not make unit
test, mainly because did not find where the String library unit tests are.
Original comment by aalto.t...@gmail.com
on 22 Jun 2014 at 8:14
And the file too...
Original comment by aalto.t...@gmail.com
on 22 Jun 2014 at 8:15
Attachments:
Looks good to me. Few comments:
1) At least in examples, preferably also in tests, it would be better to use
include equal sign in variable assignment. For example:
| ${str1} = | Convert To Lower Case | ABC |
2) Better to use `uppercase/lowercase` and not `upper case/lower case`. That's
more common way to write them and also the format used by `Should Be
Uppercase/Lowercase` keywords.
3) For consistency reasons it would be good to add also `Convert To Titlecase`
to match `Should Be Titlecase`.
Original comment by pekka.klarck
on 24 Jun 2014 at 12:59
Thanks from comments, I did not think about those ones. I did made the changes,
see the new patch for details.
Original comment by aalto.t...@gmail.com
on 24 Jun 2014 at 7:03
Attachments:
Made an assumption error (aging), have to provide new patch where titlecase
actually does what it should be. Just need to wait that my Ubuntu upgrade is
finished...
Original comment by aalto.t...@gmail.com
on 24 Jun 2014 at 7:15
OK, should be good to go.
Used string.title(), which will end in some cases to not wanted results[1]. Is
that a problem and should it be mentioned in the docs? Or should I do better
implementation?
[1]
>>> "they're bill's friends from the UK".title()
"They'Re Bill'S Friends From The Uk"
Original comment by aalto.t...@gmail.com
on 24 Jun 2014 at 7:54
Attachments:
How to convert 'ALL UPPER' to title case is actually a very good question. As
you pointed out, `'ALL UPPER'.title() == 'All Upper'` which may not be
desirable when dealing with acronyms like 'UK' or 'HTML'. For this reason Robot
doesn't itself use title() when it creates keyword names but instead converts
all first letters to upper case. We have the following options:
1) Use title() and be OK with 'convert XML to HTML' to be converted to 'Convert
Xml To Html'. As you pointed out, this ought to be documented.
2) Implement a custom solution that only upper cases first letter.
Unfortunately cannot directly use utils.printable_name because it normalizes
spaces. This would also be inconsistent with Should Be Titlecase keyword that
considers 'Convert XML To HTML' not to be titlecase.
3) Make it possible to specify should the keyword behave like 1) or 2). This is
probably an overkill solution.
4) Don't implement Convert To Titlecase at all. Nobody has requested it and I
doubt how useful it would actually be. If there are actual needs later, we can
always add it then.
I'm stating to think the last solution is the best one. At least it is the
easiest.
Original comment by pekka.klarck
on 25 Jun 2014 at 11:02
I am voting also with option 4, shall we go with that?
If we go with option 4 will you edit the patch to contain only the relevant
parts or do you want me to do a new one?
Original comment by aalto.t...@gmail.com
on 25 Jun 2014 at 2:05
Ok, let's go with option 4 and forget about Convert To Titlecase.
I can easily remove Convert To Titlecase from the patch if you want. Another
thing to change is adding `New in Robot Framework 2.8.6.` to end of the
keywords docs.
I won't commit the patch now because we are currently moving the source to
GitHub. Would you actually be interested in submitting a pull request once the
transition is done? If everything goes well, we should get source and issues
moved tomorrow.
Original comment by pekka.klarck
on 25 Jun 2014 at 2:37
GitHub sounds good for me, let's do it that way. I will assume that you will
mail to the user group when all is good to go.
Original comment by aalto.t...@gmail.com
on 25 Jun 2014 at 4:29
Robot Framework is being migrated to GitHub and issues have already been moved
to https://github.com/robotframework/robotframework/issues/. Old IDs are
preserved so you can find this particular issue using URL like:
https://github.com/robotframework/robotframework/issues/<id>
Original comment by pekka.klarck
on 30 Jun 2014 at 2:12
Original issue reported on code.google.com by
aalto.t...@gmail.com
on 19 Jun 2014 at 8:35