Hi guys,

On Thu, Aug 09, 2012 at 08:53:18PM +0200, Giuseppe Penone wrote:
> Hi Julien,
> what you mean with "when the flag image is OK"?
> 
> actually with 1 -> 5 you are scaling the flag *0.5 -> *0.9 after the
> lxpanel icons side decide by the user.
> I understand you would like to have a position 6 where the flag has the
> same size as defined on the
> panel icons size, I'll do it as soon as I'll have a couple of hours.

You could also query the exact scale factor with a spin button.

> ...
> 
> On Thu, Aug 9, 2012 at 12:01 AM, Julien Lavergne <[email protected]> wrote:
> > I pushed version 5 to trunk to get more testing.

Hm, it seems there are differences to the current version 5. Just to
clarify, Giuseppe, did you accidentally overwrite your version 5 with
what should be 6?


I have a few small comments about the code, version 0.5.10.5 on your
website. The numbers after the filename are line numbers.


xkb.h: No need to #include "ev.h" anymore.


xkb-plugin.c:269: There already is a variable called "fp" in this
function. I would prefer not to override that definition.


xkb-plugin.c:544,646,848: Do trampolines work everywhere we care? The
gcc(1) manpage says:

> A trampoline is a small piece of data or code that is created at run
> time on the stack when the address of a nested function is taken, and
> is used to call the nested function indirectly.  For some targets, it
> is made up of data only and thus requires no special treatment.  But,
> for most targets, it is made up of code and thus requires the stack to
> be made executable in order for the program to work properly.

Sofar xkb is the only plugin that requires an executable stack (see
execstack(8)). Ubuntu seems to prefer avoiding it:
https://wiki.ubuntu.com/SecurityTeam/Roadmap/ExecutableStacks
and I don't think the C standard allows them. Opinions?


xkb-plugin.c:926-928: If the system() calls fail, an
ERR("xkb: blah, blah failed\n");
message should be returned. And perhaps replace the g_printf() with
LOG(LOG_INFO, "xkb: blah, blah %s\n", str);
to be consistent.


xkb-plugin.c: Instead of 5 times on_radiobutton_flag_size_5_toggled()
you could pass the flag_size in the p_data, preferably using
GINT_TO_POINTER() and GPOINTER_TO_INT(), see
http://developer.gnome.org/glib/2.32/glib-Type-Conversion-Macros.html
But I am nitpicking. :)


xkb-plugin.c:478:13: warning: 'on_hscale_flag_size_value_changed'
defined but not used [-Wunused-function]


Black text on dark background doesn't work too well. It would be nice to
have that configurable. (Or is there a better way?)


And one bug: The plugin correctly identifies my keyboard layout as "de",
but until it is configured the config dialog only shows "us" in the
"Keyboard Layouts" section.


Thanks for the great work! It looks really awesome!

Regards,
Henry

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Lxde-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/lxde-list

Reply via email to