[Konsole-devel] KDE/kdebase/apps/konsole/src

[Diego Iastrubni]

This continues the last patch, hopefully with all your requests. I am still 
trying to understand how to set the shortcut for this action, so it's not 
included in this patch.

Sorry it took me all day, but I had to recompile  qt,kdelibs and kdebase to 
test this properly. Seems to work so far :)

On Saturday 29 March 2008 19:17:33 Diego Iastrubni wrote:
> Hi Robert,
>
> Sorry, I probably miss understood your previous message in the mailing
> list, by bad. I am replying to konsole-devel since I do want people to see
> and comment about it. I am also sending it to you directly, since I already
> lost a few mails on konsole-devel... better safe then sorry.
>
> See comments inline:
> > > <string>Enable BiDi rendering </string>
> >
> > Assuming there is enough horizontal room, could you use "Bi-directional
> > text rendering" rather than "BiDi rendering" here.
>
> Ok, it's also the text in konsole3. I will fix this.
>
> > > (valid for Arabic, Farsi or Hebrew only)
> >
> > By that I assume you mean that this setting has no effect in other
> > scripts?
>
> If I understand correctly how the BiDi algorithm works, and how it's
> implemented in scribe, that is a correct assumption. This should only
> affect the re-arrangement of "RTL" characters, it's it's not - it's a but
> in the Qt scribe implementation, or a corner case (and I don't have an
> example) which can be avoided by "enabling" the BiDi rendering - which just
> passes the original string to QPainter::drawText().
>
> I honestly don't see a problem here.
>
> > > In QPainter3, there was an option to draw a text and tell the painter
> > > "please do not implement BiDi". In QPainter4 (up to 4.4) there
> > > is no such option. I am forcing it by drawing
> > > "painter.drawText(rect,0,QChar(0x202D)+text)"
> > > (that is &LRO;), and it does seem to work,
> >
> > If you need to use a non-obvious method to do something please add
> > comments to the code to explain what you are doing and how it works.
> > This is especially true if it is a workaround for an omission in Qt, KDE
> > or some other library which might be fixed in the future.
> >
> > In this case replacing the literal "0x202D" with a suitably named
> > constant (eg. "LTR_OVERRIDE_CHAR") would also help.
>
> Ok, I will document this part of code a little more. You are right.
>
> > Where there is a lengthy discussion leading up to the change, a link to
> > the mailing list thread may help.
>
> again, I will link to that thread. In the code and in the commit log.
>
> > One small thing missing is that when adding a new property, you need to
> > explicitly set a default value for it by setting the property value in
> > the FallbackProfile class constructor in Profile.cpp.  Profiles that
> > don't customise this setting will inherit this default value from
> > FallbackProfile.
>
> Funny how I missed that in my testings :)
>
> > > quickly enable/disable it (alt+control+b in konsole3),
> > > but I am not sure what is the best way for doing it.
> >
> > The simplest method would be to add a "Bi-directional text" toggle
> > action to the View menu, just below the "Set Character Encoding" action.
>
> Are you sure you want to pollute the menus with this option? It will not be
> used that much and IMHO it will just waste space. I would like to do this
> similar to konsole3, where I could set the shortcut without a GUI element.
> This will still be visible in the tooltip and in the shortcut dialog. Well,
> at least this is the implementation I would like to see.
>
> I am not sure if I will have time to fix this today, and the working week
> starts in Sunday here (crazy country...), so worst case scenario I will be
> able to fix these issues next week. The text change should be done ASAP,
> before scripty starts it's work, otherwise about 60 teams will get to
> translate that twice. But this will still leave this patch half working, is
> this ok by you? or would you like to revert it and have a better patch by
> next week?
> _______________________________________________
> konsole-devel mailing list
> konsole-devel at kde.org
> https://mail.kde.org/mailman/listinfo/konsole-devel


-------------- next part --------------
A non-text attachment was scrubbed...
Name: konsole-bidi-part2.patch
Type: text/x-diff
Size: 2269 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20080405/7f45a893/attachment.bin>