Code review comment for lp:~kill-animals/ubuntu-terminal-app/1349749workaround-select-font-added-to-preferences

>There is no Q_INVOKABLE for setVTFont's class.

I believe that it works anyway because setVTFont is declared as a public
slot and not a public method. (some reminiscences in by brain).

>Also (I Think)
>font.family: kterm.setVTFont(jsConf.
>getFontStyle(), kterm.font.pointSize);
>Is initiated in the constructor; changing it on the fly if I recall, does
not work; you require some signal handles which are in the configs scope.

That's true but as a side effect. Writing "font.family: kterm.setVTFont()",
is read as: set to font.family what is returned by the function
kterm.setVTFont().
This is not what happens. The function kterm.setVTFont() doesn't return
anything but sets the font itself. If you want to call it this way the
proper place to put it is in Component.onCompleted.

What's wrong with "font.family: jsConf.getFontStyle()" ?

2014-09-24 14:50 GMT+02:00 Akiva <email address hidden>:

> > you are doing it is calling the invokable function
>
> This is what I don't get though, and trust me; I WAS CONFUSED
> There is no Q_INVOKABLE for setVTFont's class.
> It's invoked... SOMEHOW! but for the life of me I could not figure out
> where or how it was done.
>
> The Q_INVOKABLE macro only appears in two places:
> http://i.imgur.com/QAVVlDo.jpg
>
> Also (I Think)
> font.family: kterm.setVTFont(jsConf.getFontStyle(), kterm.font.pointSize);
> Is initiated in the constructor; changing it on the fly if I recall, does
> not work; you require some signal handles which are in the configs scope.
>
> To everything else; sounds fine though.
>
>
> T
> --
>
> https://code.launchpad.net/~akiva/ubuntu-terminal-app/1349749workaround-select-font-added-to-preferences/+merge/235621
> You are reviewing the proposed merge of
> lp:~akiva/ubuntu-terminal-app/1349749workaround-select-font-added-to-preferences
> into lp:ubuntu-terminal-app.
>