Re: [PATCH 3/4] 3 Save iconpath if icon will be used
On Sun, 27 Jan 2013, Rodolfo kix Garcia escribió: > On Fri, 25 Jan 2013, BALATON Zoltan escribió: > > > On Fri, 25 Jan 2013, Carlos R. Mafra wrote: If you want see the code. kix -- ||// //\\// Rodolfo "kix" Garcia ||\\// //\\ http://www.kix.es/ ptchs.tgz Description: application/gtar-compressed
Re: [PATCH 2/2] Use same parsing code for generated and external submenus
On Sun, 27 Jan 2013 19:16:33 + "Carlos R. Mafra" wrote: > Well, if the idea is to be picky about coding style one can comply > with wmaker's standards very easily using checkpatch.pl from the > linux kernel. Willdo once the technical stuff is agreed on... - Andreas -- BR Andreas Bierfert, M.Sc. | phone: +49 6897 1721738 | GPG: C58CF1CB andreas.bierf...@lowlatency.de | fax: +49 6897 1722828 | signed/encrypted http://lowlatency.de | cell: +49 170 9665206 | mail preferred signature.asc Description: PGP signature
Re: [Patch 0/2] Unify codepath for parsing generated and external menus
On Sun, 27 Jan 2013 19:05:29 + "Carlos R. Mafra" wrote: > Ok, thanks a lot for clarifying it. Having this symmetry on how to include > the generated menu makes sense for me. > > But how safe is this if one tries to abuse it? Say generate a huge file > to get a buffer overflow etc. I'm also worried about generating a "menu" > with 'rm -rf $HOME' or something like that. > > I usually don't think about ways to destroy things, but since WMRootMenu > is easy to edit someone might be inspired to attack wmaker users this > way. Well sure, someone could do this (the program would run in user context). But if you take a look at the current code, popen is run already, so the moving/cleaning of the code does not really change the attack vector. The output from popen is written to a growing buffer so in theory you cloud cause a oom on it. I am open to fixing any buffer overflow bugs that are pointed out. At least valgrind is happy on the loop as it is now. - Andreas -- BR Andreas Bierfert, M.Sc. | phone: +49 6897 1721738 | GPG: C58CF1CB andreas.bierf...@lowlatency.de | fax: +49 6897 1722828 | signed/encrypted http://lowlatency.de | cell: +49 170 9665206 | mail preferred signature.asc Description: PGP signature
Re: [PATCH 3/4] 3 Save iconpath if icon will be used
On Fri, 25 Jan 2013, BALATON Zoltan escribió:
> On Fri, 25 Jan 2013, Carlos R. Mafra wrote:
> >On Fri, 25 Jan 2013 at 1:06:58 +0100, Rodolfo García Peñas wrote:
> >>I changed the behavior for the winspector. Can you test it now?
> >>
> >>More testers are welcome. If the patch is ok, I will write the commit info.
> >
> >I'm a bit lost about which behavior is being fixed. What is the test
> >case to aim for?
> >
> >On the other hand, if Zoltan says it fixes his issues, I'll apply
> >without testing it myself :-)
>
> I don't have time to test it now but I've described the problem,
> testing procedure and provided a test case in the thread starting at
>
> http://lists.windowmaker.org/dev/msg04395.html
>
> The test case is here:
> http://lists.windowmaker.org/dev/msg04396.html
> (Should show a white rectangle in the app icon which is the icon window.)
>
> The procedure to test Ignore app icon is here:
> http://lists.windowmaker.org/dev/msg04452.html
>
> Hope this helps.
>
> Regards,
> BALATON Zoltan
Hi Zoltan,
I have some patches here about the icon image, related to the wmicontest tool.
Yes, you are right, the behavior of using the window_icon as dock icon should
be included. But (nothing matters before the "but" ;-) ), should we include it
as other icon image source option??? See this code:
static void set_icon_image_from_client(WIcon *icon)
{
WWindow *wwin = NULL;
if (icon && icon->owner)
wwin = icon->owner;
if (icon->icon_win != None) {
/* Get the Pixmap from the WIcon */
get_rimage_icon_from_icon_win(icon);
} else if (wwin && wwin->net_icon_image) {
/* Use _NET_WM_ICON icon */
get_rimage_icon_from_x11(icon);
} else if (wwin && wwin->wm_hints && (wwin->wm_hints->flags &
IconPixmapHint)) {
/* Get the Pixmap from the wm_hints, else, from the user */
unset_icon_image(icon);
icon->file_image = get_rimage_icon_from_wm_hints(icon);
if (!icon->file_image)
get_rimage_icon_from_user_icon(icon);
} else if (wwin && wwin->wm_hints && (wwin->wm_hints->flags &
IconWindowHint)) {
if (wwin->client_win == wwin->main_window) {
WApplication *wapp;
/* do not let miniwindow steal app-icon's icon window */
wapp = wApplicationOf(wwin->client_win);
if (!wapp || wapp->app_icon == NULL)
icon->icon_win = wwin->wm_hints->icon_window;
} else {
icon->icon_win = wwin->wm_hints->icon_window;
}
} else {
/* Get the Pixmap from the user */
get_rimage_icon_from_user_icon(icon);
}
}
These lines were in "wIconUpdate", and I added (see 3rd "else"), using the code
in the icon create for wwindow (and removing it from that function). Now the
behavior is better. Xfig runs without problem, and in wmicontest, the
icon_window is used, but we can select an icon using the winspector (now you
can't). BUT (again), if the icon is set in winspector, then the icon_image is
overriden. IMO, this new code is better, but... I would like to know your
oppinion before send the patches.
Best regards,
kix
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 2/2] Use same parsing code for generated and external submenus
On Sun, 27 Jan 2013 at 20:02:13 +0100, Rodolfo García Peñas wrote:
> On Sun, 27 Jan 2013, Andreas Bierfert escribió:
>
> > From 294b0f10829bd3c6fec0f5b50ccd451ecd076be1 Mon Sep 17 00:00:00 2001
> > From: Andreas Bierfert
> > Date: Sun, 27 Jan 2013 17:11:38 +0100
> > Subject: [PATCH 2/2] Use same parsing code for generated and external
> > submenus
> ...
> > + if(!plist)
> > + return NULL;
> ...
> > + if(!menu)
> > + return NULL;
>
> Hi Andreas!
>
> :-)
>
> and in the patch 1/2 in one place. IMO, don't try to find things using
> your eyes, use the "find" option of your editor. For example, to find
> whitespaces at the end of the lines, the find tool is needed.
Well, if the idea is to be picky about coding style one can comply
with wmaker's standards very easily using checkpatch.pl from the
linux kernel.
For example, running it in the mailbox where I saved his email (which
is not how checkpatch should be ideally used - it's meant to be run
on the outcome of 'git format-patch') generates these warnings:
[mafra@linux-ahr8:linux-2.6]$ ./scripts/checkpatch.pl ../Mail/andreas.bierfert
ERROR: patch seems to be corrupt (line wrapped?)
#61: FILE: src/rootmenu.c:981:
ame)
ERROR: spaces required around that '=' (ctx:WxV)
#65: FILE: src/rootmenu.c:984:
+ WMPropList *plist =3D NULL;
^
WARNING: Avoid CamelCase:
#65: FILE: src/rootmenu.c:984:
+ WMPropList *plist =3D NULL;
ERROR: spaces required around that '=' (ctx:WxV)
#107: FILE: src/rootmenu.c:998:
+ plist =3D WMReadPropListFromPipe(filename);
^
WARNING: Avoid CamelCase:
#107: FILE: src/rootmenu.c:998:
+ plist =3D WMReadPropListFromPipe(filename);
WARNING: suspect code indent for conditional statements (8, 12)
#111: FILE: src/rootmenu.c:999:
+ if(!plist)
+ return NULL;
ERROR: space required before the open parenthesis '('
#111: FILE: src/rootmenu.c:999:
+ if(!plist)
ERROR: spaces required around that '=' (ctx:WxV)
#116: FILE: src/rootmenu.c:1001:
+ menu =3D configureMenu(scr, plist, False);
^
WARNING: Avoid CamelCase:
#116: FILE: src/rootmenu.c:1001:
+ menu =3D configureMenu(scr, plist, False);
WARNING: Avoid CamelCase:
#116: FILE: src/rootmenu.c:1001:
+ menu =3D configureMenu(scr, plist, False);
WARNING: suspect code indent for conditional statements (8, 12)
#117: FILE: src/rootmenu.c:1002:
+ if(!menu)
+ return NULL;
ERROR: space required before the open parenthesis '('
#117: FILE: src/rootmenu.c:1002:
+ if(!menu)
ERROR: spaces required around that '=' (ctx:WxV)
#120: FILE: src/rootmenu.c:1004:
+ menu->on_destroy =3D removeShortcutsForMenu;
^
WARNING: Avoid CamelCase:
#120: FILE: src/rootmenu.c:1004:
+ menu->on_destroy =3D removeShortcutsForMenu;
ERROR: Missing Signed-off-by: line(s)
total: 8 errors, 7 warnings, 69 lines checked
../Mail/andreas.bierfert has style problems, please review.
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.4-16-gfd7b117
On Sun, 27 Jan 2013, BALATON Zoltan escribió:
> On Sun, 27 Jan 2013, "Rodolfo García Peñas (kix)" wrote:
> >The AlwaysUserIcon is saved only if the user edit the icon. I need save
> >it to the attributes file because if I save only the textbox then is
> >like the AlwaysUserIcon is set to true.
>
> Then it's a problem in your patch because it has reverted the
> default. Previously if the attribute was not present in the config
> then it was off. Unless you add the option with "No" value to
> existing configs you will change default behaviour.
No, what I said was wrong, the behaviour is not changed. If you tested the
patch, all should be like without the patch.
> >There is not more difficult to understand the attributes file, is only
> >one line more. OTOH, we can hold the last icon set in the textbox
> >without use it, for example if the user want to use the icon sometimes,
> >only open the winspector and then set or unset the flag.
> >
> >The patch is backward compatible, because if the textbox is set, then is
> >because AlwaysUserIcon is true.
>
> No this is wrong. I have this in my WMWindowAttributes:
>
> urxvt.URxvt = {
> Icon = penguin.xpm;
> };
> Firefox = {
> AlwaysUserIcon = Yes;
> Icon = "mozilla-firefox.png";
> };
>
> And only Firefox has the Ignore client supplied icon switch ticked.
> If this has changed after your patch and now URxvt also has the
> option set then there's a problem with the patch.
Zoltan, test it!! :-) All should continue ok. Please, try it, you have a lot of
knowledge about this.
> >>two cases: AlwaysUserIcon option is present (with value Yes) or it's
> >>missing instead of also having AlwaysUserIcon=No; as a third case. You
> >>might consider this too for the final version of your patch.
> >
> >But in this case, the checkbox should be removed? If the textbox is set,
> >then AlwaysUserIcon is true, and if is empty, AlwaysUserIcon is false.
>
> No. Maybe you are confused by the fact that the switch box in the
> GUI and the option in the config have names and meanings inverted
> but it's not that complicated. Basically there are these options:
>
> 1. Nothing is set: the client supplied icon is used
> 2. The Icon filename is set: the user provided icon overrides the
> client provided static icon (but if there's an icon window it's
> still preferred over the static icon because the application might
> use it to display information).
> 3. The Ignore client supplied icon option is also set: The user
> supplied icon is always preferred over all client supplied icons.
>
> Additionally the Ignore client supplied icon option is only
> meaningful if there's a user set icon so it should be deactivated
> otherwise to prevent the user to select a non-sensical option. This
> is better than giving a warning in this case.
>
> Does that make it clear or am I missing something?
I read the previous mail with Carlos, should I try to set the textbox to "grey"
or something like to avoid that the user could edit it without set the checkbox?
Quick help, how I can set the textbox to "grey" or "uneditable"?
Thanks Zoltan,
kix
> Regards,
> BALATON Zoltan
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [Patch 0/2] Unify codepath for parsing generated and external menus
On Sun, 27 Jan 2013 at 19:39:21 +0100, Andreas Bierfert wrote:
> On Sun, 27 Jan 2013 18:18:20 +
> "Carlos R. Mafra" wrote:
>
> > On Sun, 27 Jan 2013 at 18:24:08 +0100, Andreas Bierfert wrote:
> > >
> > > This way using an external menu file generated via 'wmmenugen > file'
> > > will actually do the same as running wmmenugen as command for the
> > > generated
> > > menu (which currently just gives an empty menu for me).
> >
> > Can you please be a bit more verbose about this? How does your
> > WMRootMenu look like when you use the first option ('wmmenugen > file')
> > and how are you using the second option?
> >
> > I mean, if you can copy & paste the relevant parts of your WMRootMenu
> > in these two cases that would make things easier to understand.
>
> Sure,
>
> the relevant parts are:
>
> (
> "Generated Submenu",
> OPEN_MENU,
> "|| find /usr/share/applications -type f -name '*desktop' | xargs
> wmmenugen -parser:xdg"
> ),
> ("External Submenu", OPEN_MENU, "~/wmmenugen-file"),
>
> where wmmenugen-file is generated via
>
> find /usr/share/applications -type f -name '*desktop' | xargs wmmenugen
> -parser:xdg > ~/wmmenugen-file
>
> Currently opening "External Submenu" shows a valid submenu, "Generated
> Submenu"
> does not, as the parsing codepath ist different.
>
> By adding WMReadPropListFromPipe to WINGs and simply calling getPropList as is
> done in WMReadPropListFromFile the same codepath is used for submenu
> generation
> of both menus. This actually makes "Generated Submenu" work as expected and
> more importantly making it work the same way as "External Submenu" does...
>
> Hope this makes it clearer.
Ok, thanks a lot for clarifying it. Having this symmetry on how to include
the generated menu makes sense for me.
But how safe is this if one tries to abuse it? Say generate a huge file
to get a buffer overflow etc. I'm also worried about generating a "menu"
with 'rm -rf $HOME' or something like that.
I usually don't think about ways to destroy things, but since WMRootMenu
is easy to edit someone might be inspired to attack wmaker users this
way.
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 2/2] Use same parsing code for generated and external submenus
On Sun, 27 Jan 2013, Andreas Bierfert escribió: > From 294b0f10829bd3c6fec0f5b50ccd451ecd076be1 Mon Sep 17 00:00:00 2001 > From: Andreas Bierfert > Date: Sun, 27 Jan 2013 17:11:38 +0100 > Subject: [PATCH 2/2] Use same parsing code for generated and external submenus ... > + if(!plist) > + return NULL; ... > + if(!menu) > + return NULL; Hi Andreas! :-) and in the patch 1/2 in one place. IMO, don't try to find things using your eyes, use the "find" option of your editor. For example, to find whitespaces at the end of the lines, the find tool is needed. Best regards, kix -- ||// //\\// Rodolfo "kix" Garcia ||\\// //\\ http://www.kix.es/ -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.4-16-gfd7b117
On Sun, 27 Jan 2013, "Rodolfo García Peñas (kix)" wrote:
The AlwaysUserIcon is saved only if the user edit the icon. I need save
it to the attributes file because if I save only the textbox then is
like the AlwaysUserIcon is set to true.
Then it's a problem in your patch because it has reverted the default.
Previously if the attribute was not present in the config then it was off.
Unless you add the option with "No" value to existing configs you will
change default behaviour.
There is not more difficult to understand the attributes file, is only
one line more. OTOH, we can hold the last icon set in the textbox
without use it, for example if the user want to use the icon sometimes,
only open the winspector and then set or unset the flag.
The patch is backward compatible, because if the textbox is set, then is
because AlwaysUserIcon is true.
No this is wrong. I have this in my WMWindowAttributes:
urxvt.URxvt = {
Icon = penguin.xpm;
};
Firefox = {
AlwaysUserIcon = Yes;
Icon = "mozilla-firefox.png";
};
And only Firefox has the Ignore client supplied icon switch ticked. If
this has changed after your patch and now URxvt also has the option set
then there's a problem with the patch.
two cases: AlwaysUserIcon option is present (with value Yes) or it's
missing instead of also having AlwaysUserIcon=No; as a third case. You
might consider this too for the final version of your patch.
But in this case, the checkbox should be removed? If the textbox is set,
then AlwaysUserIcon is true, and if is empty, AlwaysUserIcon is false.
No. Maybe you are confused by the fact that the switch box in the GUI and
the option in the config have names and meanings inverted but it's not
that complicated. Basically there are these options:
1. Nothing is set: the client supplied icon is used
2. The Icon filename is set: the user provided icon overrides the client
provided static icon (but if there's an icon window it's still preferred
over the static icon because the application might use it to display
information).
3. The Ignore client supplied icon option is also set: The user supplied
icon is always preferred over all client supplied icons.
Additionally the Ignore client supplied icon option is only meaningful if
there's a user set icon so it should be deactivated otherwise to prevent
the user to select a non-sensical option. This is better than giving a
warning in this case.
Does that make it clear or am I missing something?
Regards,
BALATON Zoltan
Re: [repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.4-16-gfd7b117
On Sun, 27 Jan 2013, Carlos R. Mafra wrote: My understanding about the warning is that it tries to warn the user about a mistake he's doing. Yes but if you don't allow the user to make a mistake at the first place then there's no need for a warning. This is less confusing for the user than throwing a warning at him and letting him figure out what mistake he did. The user is saying "I want wmaker to ignore the client icon and use the icon I set up for it". But at the same time he's not saying which icon wmaker is supposed to use instead (ie empty textbox). So the user should only be able to say this when it's meaningful to say so (i.e. there's an icon set and not otherwise). So I think the option should only be enabled when the file textbox is set and disabled if the user removes the file. No, the option should be enabled when the user ticks the option and disabled otherwise. It's supposed to be simple and have no correlation to the textbox. However, if the user ticks the 'ignore' option but does not define which icon to use, that's a mistake and it better be warned. No? I think there's a misunderstanding here. By disabled I mean "greyed out", not selectable, inactive, etc. (cf. WMSetButtonEnabled and WMSetButtonSelected). The Ignore option and thus the switch button does have a correlation to the textbox as it's only meaningful to select it if there's an icon set. So it should not be active if it's invalid to select it. Does this make sense? Regards, BALATON Zoltan -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [Patch 0/2] Unify codepath for parsing generated and external menus
On Sun, 27 Jan 2013 18:18:20 +
"Carlos R. Mafra" wrote:
> On Sun, 27 Jan 2013 at 18:24:08 +0100, Andreas Bierfert wrote:
> >
> > This way using an external menu file generated via 'wmmenugen > file'
> > will actually do the same as running wmmenugen as command for the generated
> > menu (which currently just gives an empty menu for me).
>
> Can you please be a bit more verbose about this? How does your
> WMRootMenu look like when you use the first option ('wmmenugen > file')
> and how are you using the second option?
>
> I mean, if you can copy & paste the relevant parts of your WMRootMenu
> in these two cases that would make things easier to understand.
Sure,
the relevant parts are:
(
"Generated Submenu",
OPEN_MENU,
"|| find /usr/share/applications -type f -name '*desktop' | xargs wmmenugen
-parser:xdg"
),
("External Submenu", OPEN_MENU, "~/wmmenugen-file"),
where wmmenugen-file is generated via
find /usr/share/applications -type f -name '*desktop' | xargs wmmenugen
-parser:xdg > ~/wmmenugen-file
Currently opening "External Submenu" shows a valid submenu, "Generated Submenu"
does not, as the parsing codepath ist different.
By adding WMReadPropListFromPipe to WINGs and simply calling getPropList as is
done in WMReadPropListFromFile the same codepath is used for submenu generation
of both menus. This actually makes "Generated Submenu" work as expected and
more importantly making it work the same way as "External Submenu" does...
Hope this makes it clearer.
Regards,
Andreas
--
BR Andreas Bierfert, M.Sc. | phone: +49 6897 1721738 | GPG: C58CF1CB
andreas.bierf...@lowlatency.de | fax: +49 6897 1722828 | signed/encrypted
http://lowlatency.de | cell: +49 170 9665206 | mail preferred
signature.asc
Description: PGP signature
Re: [Patch 0/2] Unify codepath for parsing generated and external menus
On Sun, 27 Jan 2013 at 18:24:08 +0100, Andreas Bierfert wrote:
>
> This way using an external menu file generated via 'wmmenugen > file'
> will actually do the same as running wmmenugen as command for the generated
> menu (which currently just gives an empty menu for me).
Can you please be a bit more verbose about this? How does your
WMRootMenu look like when you use the first option ('wmmenugen > file')
and how are you using the second option?
I mean, if you can copy & paste the relevant parts of your WMRootMenu
in these two cases that would make things easier to understand.
I'm also explicitly Cc'ing Christophe Curis in the hope that he
can share his thoughts about this :-)
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.4-16-gfd7b117
On 27/01/2013 17:03, BALATON Zoltan wrote: > On Sat, 26 Jan 2013, crmafra wrote: >>If the user tries to set an icon (ignore flag is set) but the file >> textbox is not set we show the >>warning: >> >>"Ignore client supplied icon is set, but icon filename textbox is >> empty. Using client supplied icon" >> >>and use the client-supplied icon instead. > > Why not just make sure the option is only enabled when there's a file > set and have it disabled and cleared otherwise? Having an option which > only results in a warning if the user tries to enable it is not a good > UI design. So I think the option should only be enabled when the file > textbox is set and disabled if the user removes the file. > > I can't comment on Rodolfo's code changes because I can't follow the > icon code so I try to only comment on the behaviour. > > I also saw a log message that said that the AlwaysUserIcon option is now > saved always. That's bad in my opinion because it just makes config > files harder to read without helping you in any way because the user > will have old config files without this option so you should handle the > case when the option is missing anyway. Thus it's better to only have Hi Zoltan, Carlos, The AlwaysUserIcon is saved only if the user edit the icon. I need save it to the attributes file because if I save only the textbox then is like the AlwaysUserIcon is set to true. There is not more difficult to understand the attributes file, is only one line more. OTOH, we can hold the last icon set in the textbox without use it, for example if the user want to use the icon sometimes, only open the winspector and then set or unset the flag. The patch is backward compatible, because if the textbox is set, then is because AlwaysUserIcon is true. > two cases: AlwaysUserIcon option is present (with value Yes) or it's > missing instead of also having AlwaysUserIcon=No; as a third case. You > might consider this too for the final version of your patch. But in this case, the checkbox should be removed? If the textbox is set, then AlwaysUserIcon is true, and if is empty, AlwaysUserIcon is false. Best Regards, kix > Regards, > BALATON Zoltan > > -- ||// //\\// Rodolfo "kix" Garcia ||\\// //\\ http://www.kix.es/ -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [Patch 0/2] Unify codepath for parsing generated and external menus
On 27/01/2013 18:24, Andreas Bierfert wrote:
> Hi,
>
> the following two patches change the parsing for generated menus
> from external commands to use the same code path as externally
> saved menus.
>
> This way using an external menu file generated via 'wmmenugen >
> file' will actually do the same as running wmmenugen as command for
> the generated menu (which currently just gives an empty menu for
> me).
>
> Regards, Andreas
>
Hi,
I didn't test your patches :-(
Some comments about the coding style.
1. The "if{" should be "if {"
2. The "else" in patch 1 should include the bracket "{".
+ pldata->ptr[0] = '\0';
+ }
+ else
+ pldata->ptr = wrealloc(pldata->ptr,
Cheers,
kix
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[PATCH 2/2] Use same parsing code for generated and external submenus
From 294b0f10829bd3c6fec0f5b50ccd451ecd076be1 Mon Sep 17 00:00:00 2001
From: Andreas Bierfert
Date: Sun, 27 Jan 2013 17:11:38 +0100
Subject: [PATCH 2/2] Use same parsing code for generated and external submenus
This patch changes the behavior of generated menues to use the same parsing
code for proplists (getPropList) as the extern submenu does. This way a call
to 'wmmenugen > file' gives the same result as adding that call to a generated
submenu.
---
src/rootmenu.c | 44
1 file changed, 8 insertions(+), 36 deletions(-)
diff --git a/src/rootmenu.c b/src/rootmenu.c
index 2fcd0ae..422b2dd 100644
--- a/src/rootmenu.c
+++ b/src/rootmenu.c
@@ -982,10 +982,8 @@ static WMenu *readMenuFile(WScreen * scr, char *file_name)
static WMenu *readMenuPipe(WScreen * scr, char **file_name)
{
+ WMPropList *plist = NULL;
WMenu *menu = NULL;
- FILE *file = NULL;
- WMenuParser parser;
- char *command, *params, *shortcut, *title;
char *filename;
char flat_file[MAXLINE];
int i;
@@ -998,42 +996,16 @@ static WMenu *readMenuPipe(WScreen * scr, char
**file_name)
}
filename = flat_file + (flat_file[1] == '|' ? 2 : 1);
- file = popen(filename, "r");
- if (!file) {
- werror(_("%s:could not open menu file"), filename);
- return NULL;
- }
- parser = WMenuParserCreate(flat_file, file, DEF_CONFIG_PATHS);
- menu_parser_register_macros(parser);
-
- while (WMenuParserGetLine(parser, &title, &command, ¶ms,
&shortcut)) {
-
- if (command == NULL || !command[0]) {
- WMenuParserError(parser, _("missing command in menu
config") );
- freeline(title, command, params, shortcut);
- break;
- }
- if (strcasecmp(command, "MENU") == 0) {
- menu = wMenuCreate(scr, M_(title), True);
- menu->on_destroy = removeShortcutsForMenu;
- if (!parseCascade(scr, menu, parser)) {
- wMenuDestroy(menu, True);
- menu = NULL;
- }
- freeline(title, command, params, shortcut);
- break;
- } else {
- WMenuParserError(parser, _("no title given for the root
menu") );
- freeline(title, command, params, shortcut);
- break;
- }
+ plist = WMReadPropListFromPipe(filename);
- freeline(title, command, params, shortcut);
- }
+ if(!plist)
+ return NULL;
- WMenuParserDelete(parser);
- pclose(file);
+ menu = configureMenu(scr, plist, False);
+ if(!menu)
+ return NULL;
+ menu->on_destroy = removeShortcutsForMenu;
return menu;
}
--
1.8.1
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[PATCH 1/2] WINGS: New function WMReadPropListFromPipe
From 67650e22a75f5ffd90a74ad502f56d5c315ab2a2 Mon Sep 17 00:00:00 2001
From: Andreas Bierfert
Date: Sun, 27 Jan 2013 17:11:01 +0100
Subject: [PATCH 1/2] WINGS: New function WMReadPropListFromPipe
This functions reads a proplist from a pipe instead of a file (like
WMReadPropListFromFile does). It uses a call to popen to open the desired
command, reads data into a buffer till EOF and passes the data to getPropList
for parsing.
---
WINGs/WINGs/WUtil.h | 2 ++
WINGs/proplist.c| 54 +
2 files changed, 56 insertions(+)
diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h
index 30476e0..84f60b9 100644
--- a/WINGs/WINGs/WUtil.h
+++ b/WINGs/WINGs/WUtil.h
@@ -807,6 +807,8 @@ char* WMGetPropListDescription(WMPropList *plist, Bool
indented);
WMPropList* WMReadPropListFromFile(char *file);
+WMPropList* WMReadPropListFromPipe(char *command);
+
Bool WMWritePropListToFile(WMPropList *plist, char *path);
/* ---[ WINGs/userdefaults.c ] */
diff --git a/WINGs/proplist.c b/WINGs/proplist.c
index 7b0f49d..04bd909 100644
--- a/WINGs/proplist.c
+++ b/WINGs/proplist.c
@@ -1545,6 +1545,60 @@ WMPropList *WMReadPropListFromFile(char *file)
return plist;
}
+WMPropList *WMReadPropListFromPipe(char *command)
+{
+ FILE *file;
+ WMPropList *plist;
+ PLData *pldata;
+ char line[1024];
+
+ file = popen(command, "r");
+
+ if (!file) {
+ werror(_("%s:could not open menu file"), command);
+ return NULL;
+ }
+
+ pldata = (PLData *) wmalloc(sizeof(PLData));
+ pldata->ptr = NULL;
+ pldata->filename = command;
+ pldata->lineNumber = 1;
+
+ /* read from file till EOF or OOM and fill proplist buffer*/
+ while(fgets(line, sizeof(line), file) != NULL) {
+ if(pldata->ptr == NULL) {
+ pldata->ptr = wmalloc(strlen(line)+1);
+ pldata->ptr[0] = '\0';
+ }
+ else
+ pldata->ptr = wrealloc(pldata->ptr,
+ strlen(line) + strlen(pldata->ptr) + 1);
+
+ pldata->ptr = strncat(pldata->ptr, line, strlen(line));
+ }
+
+ pclose(file);
+
+ plist = getPropList(pldata);
+
+ if (getNonSpaceChar(pldata) != 0 && plist) {
+ COMPLAIN(pldata, _("extra data after end of property list"));
+ /*
+* We can't just ignore garbage after the end of the description
+* (especially if the description was read from a file), because
+* the "garbage" can be the real data and the real garbage is in
+* fact in the beginning of the file (which is now inside plist)
+*/
+ WMReleasePropList(plist);
+ plist = NULL;
+ }
+
+ wfree(pldata->ptr);
+ wfree(pldata);
+
+ return plist;
+}
+
/* TODO: review this function's code */
Bool WMWritePropListToFile(WMPropList * plist, char *path)
--
1.8.1
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[Patch 0/2] Unify codepath for parsing generated and external menus
Hi, the following two patches change the parsing for generated menus from external commands to use the same code path as externally saved menus. This way using an external menu file generated via 'wmmenugen > file' will actually do the same as running wmmenugen as command for the generated menu (which currently just gives an empty menu for me). Regards, Andreas -- BR Andreas Bierfert, M.Sc. | phone: +49 6897 1721738 | GPG: C58CF1CB andreas.bierf...@lowlatency.de | fax: +49 6897 1722828 | signed/encrypted http://lowlatency.de | cell: +49 170 9665206 | mail preferred signature.asc Description: PGP signature
Re: [repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.4-16-gfd7b117
On Sun, 27 Jan 2013 at 17:03:16 +0100, BALATON Zoltan wrote: > On Sat, 26 Jan 2013, crmafra wrote: > > If the user tries to set an icon (ignore flag is set) but the file > > textbox is not set we show the > > warning: > > > > "Ignore client supplied icon is set, but icon filename textbox is empty. > > Using client supplied icon" > > > > and use the client-supplied icon instead. > > Why not just make sure the option is only enabled when there's a > file set and have it disabled and cleared otherwise? Having an > option which only results in a warning if the user tries to enable > it is not a good UI design. My understanding about the warning is that it tries to warn the user about a mistake he's doing. The user is saying "I want wmaker to ignore the client icon and use the icon I set up for it". But at the same time he's not saying which icon wmaker is supposed to use instead (ie empty textbox). > So I think the option should only be enabled when the file textbox is > set and disabled if the user removes the file. No, the option should be enabled when the user ticks the option and disabled otherwise. It's supposed to be simple and have no correlation to the textbox. However, if the user ticks the 'ignore' option but does not define which icon to use, that's a mistake and it better be warned. No? -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.4-16-gfd7b117
On Sat, 26 Jan 2013, crmafra wrote: If the user tries to set an icon (ignore flag is set) but the file textbox is not set we show the warning: "Ignore client supplied icon is set, but icon filename textbox is empty. Using client supplied icon" and use the client-supplied icon instead. Why not just make sure the option is only enabled when there's a file set and have it disabled and cleared otherwise? Having an option which only results in a warning if the user tries to enable it is not a good UI design. So I think the option should only be enabled when the file textbox is set and disabled if the user removes the file. I can't comment on Rodolfo's code changes because I can't follow the icon code so I try to only comment on the behaviour. I also saw a log message that said that the AlwaysUserIcon option is now saved always. That's bad in my opinion because it just makes config files harder to read without helping you in any way because the user will have old config files without this option so you should handle the case when the option is missing anyway. Thus it's better to only have two cases: AlwaysUserIcon option is present (with value Yes) or it's missing instead of also having AlwaysUserIcon=No; as a third case. You might consider this too for the final version of your patch. Regards, BALATON Zoltan -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
