Opened 9 years ago

Closed 9 years ago

#5563 closed enhancement (fixed)

[Patch] ShowImage app localization

Reported by: mt Owned by: stippi
Priority: normal Milestone: R1
Component: Kits/Locale Kit Version: R1/Development
Keywords: ShowImage Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Localize GUI strings for ShowImage app. Patch and en.catkeys added.

Attachments (4)

ShowImage.patch (34.1 KB) - added by mt 9 years ago.
patch file
en.catkeys (2.7 KB) - added by mt 9 years ago.
ShowImage.2.patch (34.5 KB) - added by mt 9 years ago.
Second patch
ShowImageUpdate.patch (3.1 KB) - added by Karvjorm 9 years ago.
Another localization update for ShowImage app.

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by mt

Attachment: ShowImage.patch added

patch file

Changed 9 years ago by mt

Attachment: en.catkeys added

comment:1 Changed 9 years ago by stippi

Owner: changed from pulkomandy to stippi
Status: newin-progress

The patch looks pretty good, thanks a lot!

Just some possible improvements for future patches:

  • You could include ControlLook.h and use be_control_look->DefaultItemSpacing() instead of hardcoding 5 or so for layout item spacing. I.e. "float spacing = be_control_look->DefaultItemSpacing();" once and then using "spacing" everywhere.
  • The bs_printf() function has some tabs or weird spacing between type and variable names.
  • The coding cleanups are greatly appreciated. In some places you could also improve variable names to be slightly more descriptive, but it would certainly not prevent me from applying this patch.

Thanks for the good work you are doing, mt. IMHO you should be proposed for commit access soon. :-)

comment:2 Changed 9 years ago by mt

I applied be_control_look->DefaultItemSpacing() and modified bs_printf(). please review.

Changed 9 years ago by mt

Attachment: ShowImage.2.patch added

Second patch

comment:3 Changed 9 years ago by stippi

Resolution: fixed
Status: in-progressclosed

Thanks a bunch! Applied in hrev35929. Don't know what to do with en.catkeys. Isn't that file auto-generated?

comment:4 in reply to:  3 Changed 9 years ago by mt

Replying to stippi:

Thanks a bunch! Applied in hrev35929. Don't know what to do with en.catkeys. Isn't that file auto-generated?

Thanks for applying patch. I added en.catkeys on this ticket for reference, so it does not need to apply to Haiku tree.

Changed 9 years ago by Karvjorm

Attachment: ShowImageUpdate.patch added

Another localization update for ShowImage app.

comment:5 Changed 9 years ago by Karvjorm

Resolution: fixed
Status: closedreopened

I found some strings not yet localized in ShowImage app. I compiled ShowImageUpdate.patch to fix these. This is my first patch, so I do not know, if I compiled it correct.

comment:6 Changed 9 years ago by stippi

The patch is the right direction in principle, but the string composition needs to be changed. One sentence with a variable in it should be translatable as one sentence, so other translations can put the variable at any place. I know this is still possible if the sentence is composed of just three parts, but it's not good style to keep it like that. It can be changed for example by using a stack allocated buffer and using snprintf().

comment:7 Changed 9 years ago by stippi

Resolution: fixed
Status: reopenedclosed

Fixed the issues myself and applied the patch in hrev36647, thanks!

Note: See TracTickets for help on using tickets.