On 08/ 4/10 02:56 AM, Takeshi Asano wrote:
Hi Keith,
Thank you, and sorry for delayed response since I was out
for 1.5 days.
Regarding the test case update, I see your points and
will revise the webrev by incorporating them.
Regarding the "left", "right", "center", I'll make
them constants as you suggested, to make inside of i18n.py better.
But regarding whether to eliminate ljust_columns() etc.,
I thought it might be simpler to keep them to align
with standard ljust/rjust/center methods. How do you think?
I'm fine with leaving them in (actually, I'd prefer that they *do* get
left in).
- Keith
Thanks,
Takeshi
On 2010年08月03日 01:05, Keith Mitchell wrote:
On 07/30/10 02:43 AM, Takeshi Asano wrote:
Hi Keith,
I've revised the webrev:
http://cr.opensolaris.org/~asano/slim-text-16655-v2/
Changes from previous version are:
- test/test_i18n.py
created
The tests look good, but miss a few edge cases. Would you mind adding
tests for the following scenarios?
* charwidth/textwidth of "str" object (as opposed to unicode objects,
which are tested)
* textwidth of unicode string with tab(s)
* fit_text_truncate with fillchar as a multi-column character (use
self.assertRaises(...))[1]
One extra note: If you have time, please define "left", "right",
"center" as constants at the top of i18n.py, so that consumers can do
something like "i18n.fit_text_truncate(text, width,
just=i18n.CENTER)". If there's no time, the existence of the separate
ljust/rjust/center_columns functions is good enough.
[1]http://docs.python.org/library/unittest.html#unittest.TestCase.assertRaises
- convert_paragraph() in i18n.py
modified to not count leading white space
- add_paragraph() in inner_window.py
modified handling of border_size[1] in calculating
max_chars (described below)
The bug I mentioned yesterday was actually in add_paragraph
rather than in convert_paragraph.
Due to different handling of self.border_size[1] between
add_paragraph and add_text during calculation of max_chars,
last character of line sometimes cut off.
(e.g. add_paragraph's max_chars is 75 while add_text's one is 74).
add_paragraph looks to assume border_size[1] holds sum
of "both sides" borders while add_text looks to assume
"each side". Since I see case that border_size[1] is 3,
I assume add_text's way is right and modified add_paragraph.
Please let me know if it's wrong.
Thanks for catching that! Yes, border_size is for "each side," so the
modification you've done is correct.
So far I kept border_size=(0,2) for central_area unchanged, but
may have separate request in the future to change it to (0,1)
in case more room is needed.
That seems reasonable. It could also be set dynamically based on the
needs of a given translation.
Thanks,
Keith
Thanks,
Takeshi
On 2010年07月29日 18:38, Takeshi Asano wrote:
Thank you, Keith and Dave.
I see, I wrote test/test_i18n.py.
But I found bug in convert_paragraph() as well so please let me revise
the webrev in a couple of days which will covert both addition of the
test script and fix for the bug.
Thanks,
Takeshi
On 2010年07月29日 02:49, Keith Mitchell wrote:
Excellent point. It seems I'm the worst at remembering things like
copyright and unit tests.
Takeshi, can you provide some PyUnit tests for the i18n.py
functions? They should be straightforward.
- Keith
On 07/28/10 10:30 AM, Dave Miner wrote:
I'm surprised to not see any unit tests for the new module...
Dave
On 07/28/10 11:28 AM, Keith Mitchell wrote:
Hi Takeshi,
The changes looked good to me when I reviewed privately.
Since I'll be handling the push of the bundle, one more set of eyes
would be appreciated.
- Keith
On 07/27/10 06:00 PM, Takeshi Asano wrote:
Hi all,
Could you please review fix for text-install i18n issues:
bug: https://defect.opensolaris.org/bz/show_bug.cgi?id=16655
webrev: http://cr.opensolaris.org/~asano/slim-text-16655/
Thanks,
Takeshi
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
