Re: [PATCH 3/4] 3 Save iconpath if icon will be used
Hello, On Sun, 27 Jan 2013, Rodolfo García Peñas wrote: better, but... I would like to know your oppinion before send the patches. My opinion is as before: code cleanup patches should not change behaviour so everything should work the same after your patches as they worked before them. (The last behaviour changing patch for icon.c selection seems to be ee1f13da45a8f3371a0dfe4a2a636c402ddf7b3d and the order is described in that commit message.) I can't offer help in checking code changes because I don't understand your code and don't have time to get knowledgeable about this part of Window Maker. So don't wait for comments about patches from me. The test case, test procedure and descriptions in my previous messages of how I think it should work were the best I could offer I'm afraid. Regards, BALATON Zoltan
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 3/4] 3 Save iconpath if icon will be used
On 25/01/2013 11:53, BALATON Zoltan wrote:
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 Carlos, Zoltan,
I am changing the icon behavior in winspector.c, these cases:
1. ignore flag + file set
2. no ignore flag + file set
3. ignore flag + no file set
4. no ignore flag + no file set
I didn't change the xfig related problem.
About the patch, we have 8 different cases, 4 for apply and 4 for save.
Apply:
1. ignore flag + file set
2. no ignore flag + file set
3. ignore flag + no file set
4. no ignore flag + no file set
If the ignore client supplied icon flag is not set, then the user wants to
use the client (window/icon) provided image, so the file textbox don't matter.
Then, we use the client icon and we don't use the file provided in the text
box. Options 2 and 4 are done.
If the user set the flag, and set the icon name, perfect, the user provides the
icon. Option 1 is done.
The problem is if the user try to set an icon (ignore flag is set) but the file
textbox is not set (no icon provided). Now the code does:
1. Show a message box to the user with a warning. The warning show:
+ snprintf(buf, len, _(Ignore client supplied icon is
set, but icon filename textbox is empty. Using client supplied icon));
2. Unset the ignore flag and use the client provided icon. Is like we change
the case 3 to case 4:
Option 3 is done too.
This is the full code:
+ /* If always_user_icon flag is set, but the user icon is not set
+* we use client supplied icon and we unset the flag */
+ if ((WFLAGP(wwin, always_user_icon) (!file))) {
+ /* Show the warning */
+ char *buf;
+ int len = 100;
+ buf = wmalloc(len);
+ snprintf(buf, len, _(Ignore client supplied icon is
set, but icon filename textbox is empty. Using client supplied icon));
+ wMessageDialog(panel-frame-screen_ptr, _(Warning),
buf, _(OK), NULL, NULL);
+ wfree(buf);
+ wfree(file);
+
+ /* Change the flags */
+ WSETUFLAG(wwin, always_user_icon, 0);
+ WMSetButtonSelected(panel-alwChk, 0);
+ }
After these lines, we change the initial case 3 to the case 4, and then we can
apply the cases 1,2,4:
+ /* After test the always_user_icon flag value before,
+* the else block is used only if the flag is set and
+* the icon text box has an icon path */
+ if (!WFLAGP(wwin, always_user_icon)) { *- Cases 1,3*
+ /* Change App Icon image, using the icon provided by
the client */
if (wapp-app_icon)
+ wIconUpdate(wapp-app_icon-icon,
+
get_rimage_icon_from_wm_hints(wapp-app_icon-icon));
+
+ /* Change icon image if the app is minimized,
+* using the icon provided by the client */
+ if (wwin-icon)
+ wIconUpdate(wwin-icon,
get_rimage_icon_from_wm_hints(wwin-icon));
} else { *- Case 4 (and wrong 3*
/* Change App Icon image */
if (wapp-app_icon)
+ wIconChangeImageFile(wapp-app_icon-icon,
file);
/* Change icon image if the app is minimized */
if (wwin-icon)
+ wIconChangeImageFile(wwin-icon, file);
}
About the save, we do the same in the 4 options:
Now all is easier. We save the flag Yes or No always and we save the user
provided icon in the textbox.
Then, the user can see the last icon set in the textbox by the user always, if
the icon is used (flag is set) or not.
+ /* Save the icon info */
+ /* The flag Ignore client suplied icon is not selected */
+ buf1 = wmalloc(4);
+ snprintf(buf1, 4, %s,
Re: [PATCH 3/4] 3 Save iconpath if icon will be used
On Sat, 26 Jan 2013 at 11:12:01 +0100, Rodolfo García Peñas (kix) wrote: Thanks for the detailed explanation. On 25/01/2013 11:53, BALATON Zoltan wrote: The procedure to test Ignore app icon is here: http://lists.windowmaker.org/dev/msg04452.html I am changing the icon behavior in winspector.c, these cases: 1. ignore flag + file set 2. no ignore flag + file set 3. ignore flag + no file set 4. no ignore flag + no file set I didn't change the xfig related problem. Zoltan's test scenario has a lot of things to watch... The problem is if the user try to set an icon (ignore flag is set) but the file textbox is not set (no icon provided). Now the code does: 1. Show a message box to the user with a warning. The warning show: + snprintf(buf, len, _(Ignore client supplied icon is set, but icon filename textbox is empty. Using client supplied icon)); 2. Unset the ignore flag and use the client provided icon. Is like we change the case 3 to case 4: This change looks correct to me. I'll apply your patch and do some copy paste from your explanation above as the commit message. Thanks a lot for keeping track of this. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
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,
about this problem, after some tests, I think the right code is:
8---
kix@kentin:~/src/wmaker/git/wmaker-crm/src$ git diff
diff --git a/src/icon.c b/src/icon.c
index 1a23b88..fdc377d 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -126,8 +126,6 @@ WIcon *icon_create_for_wwindow(WWindow *wwin)
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;
}
}
#ifdef NO_MINIWINDOW_TITLES
8---
It runs with the wmicontest application, and run with xfig. Xfig can now use
the user icon, or the client icon. What do you think?
kix
--
||// //\\// Rodolfo kix Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 3/4] 3 Save iconpath if icon will be used
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 :-) -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH 3/4] 3 Save iconpath if icon will be used
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
Re: [PATCH 3/4] 3 Save iconpath if icon will be used
On Thu, 29 Nov 2012, BALATON Zoltan escribió:
On Thu, 29 Nov 2012, Rodolfo Garc?a Pe?as (kix) wrote:
Yes, you are right. We have four options:
1. ignore flag + file set
2. no ignore flag + file set
3. ignore flag + no file set
4. no ignore flag + no file set
Option 1 is clear, we provide a new icon and ignore the supplied
icon by the client.
Option 4 is clear, we use the client supplied icon and the icon
provided is not used.
But, what about options 2 and 3?
Option 3 is fairly clear too. If there is no user set icon you
either ignore the Ignore flag (bacause it makes no sense) or you may
use default app icon to override the icon window but I'm not sure
how useful is that.
For option 2 I already said what I think should happen: use client
supplied icon _window_ or user supplied icon otherwise.
If we provide icon, should the flag ignored?
No, there's a reason why this flag is there. If it could be ignored
there would not be such a flag.
If we check the flag, should the file ignored?
No, the flag says AlwaysUserIcon. (That is ignore the client
supplied icons of any sorts and always use the icon file instead.)
Should we can remove the check? is the work done by the text
field? If the text is empty, should the icon provided by the
client used?
But why should we change anything? Could you just make everything do
the same that it used to do before you've started changing it? Your
patches were meant to be clean up so they should preserve
functionality and not change behaviour. Or did I get that wrong? Was
there a problem you've tried to fix other than cleaning up code?
IMO we are doing something wrong with these applications (like
xfig). Why these applications have that behaviour? Xfig don't use
title in the miniwindow, sets their own icon,...
I think it is because the Ignore client supplied icon flag is broken
and not used even if it's set in WMWindowAttributes. Since xfig
provides an icon window it will override the icon even when the
Ignore flag is set (or even if the AlwaysUserIcon flag is set in the
config because the checkbox in the GUI is also detached from what's
in the config file).
Again, if we remove these lines in create_for_wwindow (icon.c)
xfig runs like other applications. IMHO, these lines should be
removed:
121 if (wwin-wm_hints (wwin-wm_hints-flags IconWindowHint)) {
122 if (wwin-client_win == wwin-main_window) {
123 WApplication *wapp;
124 /* do not let miniwindow steal app-icon's
icon window */
125 wapp = wApplicationOf(wwin-client_win);
126 if (!wapp || wapp-app_icon == NULL)
127 icon-icon_win = wwin-wm_hints-icon_window;
128 } else {
129 icon-icon_win = wwin-wm_hints-icon_window;
130 }
131 }
Try it, and you will see that xfig has a common behaviour. The
application has icon, the title,... These lines override the
common icon management.
But this breaks the wmicontest test case I've sent earlier and
probably also dockapps which need to have an icon window.
Probably we should check the icon behaviour to see if the current
code includes the idea of these lines, but, IMO this lines breaks
the flow for icon management, creating a shortcut with a different
flow and should be removed.
I don't get this. Are you trying to get rid of a special case based
only on aesthetic reasons? There is a reason that this special case
is there (as stated in the comment) so it's needed even if it's not
nice to have it there. You could move it elsewhere but just deleting
it would break something.
Again, thanks a lot for your comments. Please, try the patches, I
think now is better.
I've tried them and it does not seem better to me. Actually, it got
worse. Try to do the test sequence with xcalc. With current next
branch if there's nothing in the config it has no app icon (uses the
default icon) only the miniwindow has an icon. Then the icon from
the cache is still saved and the Ignore option checked wrongly
mismatching the config file. I gave up at this point.
Regards,
BALATON Zoltan
Hi Zoltan!
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.
Cheers,
kix
--
||// //\\// Rodolfo kix Garcia
||\\// //\\ http://www.kix.es/
From e69d230a8c1fdb2aa43fc0c765bfac3c85ab7fa6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)?= k...@kix.es
Date: Fri, 25 Jan 2013 00:59:08 +0100
Subject: [PATCH] New winspector behavior
For testing only.
---
src/winspector.c | 81 --
1 file changed, 48 insertions(+), 33 deletions(-)
diff --git a/src/winspector.c b/src/winspector.c
index 7335030..8d5923a 100644
--- a/src/winspector.c
+++ b/src/winspector.c
@@ -475,29 +475,23
[PATCH 3/4] 3 Save iconpath if icon will be used
From 244671a650bae1be4d2b12d0ecc308c63b08783d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)?= k...@kix.es
Date: Wed, 28 Nov 2012 23:56:19 +0100
Subject: [PATCH 3/4] 3 Save iconpath if icon will be used
This patch avoid to save the icon name in the configuration file
if the flag Ignore client suplied icon is not selected.
First, when winspector try to show the icon to the user, mustn't search
the default icon. It should use the specific icon or the icon provided
by the client.
When we click in save Settings, the function saveSettings reads the icon
specified by the user in the text box. If the checkbox Ignore client supplied
icon is not selected, then saves the icon too.
Using this behaviour we never recover the initial position (no icon selected,
no ignore client supplied icon flag set). This patch recovers the initial
state of the icon options.
When the user unset the ignore client the filename provided is not saved in
the configuration file. Now, the configuration is like the initial status.
If the user checks the flag and set the file to use as icon, the file is used.
---
src/winspector.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/src/winspector.c b/src/winspector.c
index 543b764..239f7f6 100644
--- a/src/winspector.c
+++ b/src/winspector.c
@@ -314,8 +314,8 @@ static int showIconFor(WMScreen *scrPtr, InspectorPanel
*panel, char *wm_instanc
file = NULL;
}
} else {
- /* Get the application icon, default included */
- db_icon = wDefaultGetIconFile(wm_instance, wm_class, True);
+ /* Get the application icon, default NOT included */
+ db_icon = wDefaultGetIconFile(wm_instance, wm_class, False);
if (db_icon != NULL)
file = wstrdup(db_icon);
}
@@ -433,7 +433,7 @@ static void saveSettings(WMButton *button, InspectorPanel
*panel)
WWindow *wwin = panel-inspected;
WDDomain *db = WDWindowAttributes;
WMPropList *dict = NULL;
- WMPropList *winDic, *appDic, *value, *key = NULL, *key2;
+ WMPropList *winDic, *appDic, *value, *value1, *key = NULL, *key2;
char *icon_file, *buf1, *buf2;
int flags = 0, i = 0, different = 0, different2 = 0;
@@ -475,16 +475,29 @@ static void saveSettings(WMButton *button, InspectorPanel
*panel)
winDic = WMCreatePLDictionary(NULL, NULL);
appDic = WMCreatePLDictionary(NULL, NULL);
- /* Update icon for window */
- icon_file = WMGetTextFieldText(panel-fileText);
- if (icon_file) {
- if (icon_file[0] != 0) {
- value = WMCreatePLString(icon_file);
- different |= insertAttribute(dict, winDic, AIcon,
value, flags);
- different2 |= insertAttribute(dict, appDic, AIcon,
value, flags);
- WMReleasePropList(value);
+ /* If the Ignore client suplied icon is not selected flag was not set,
+* then, don't save the icon filename. If saved, the application will
use
+* that icon, even the flag is not set. */
+ if (WMGetButtonSelected(panel-alwChk) != 0) {
+ /* Update icon for window */
+ icon_file = WMGetTextFieldText(panel-fileText);
+ if (icon_file) {
+ if (icon_file[0] != 0) {
+ value = WMCreatePLString(icon_file);
+ different |= insertAttribute(dict, winDic,
AIcon, value, flags);
+ different2 |= insertAttribute(dict, appDic,
AIcon, value, flags);
+ WMReleasePropList(value);
+
+ /* Set the ckeck for AAlwaysUserIcon only if
icon_file exists */
+ buf1 = wmalloc(4);
+ snprintf(buf1, 4, %s,
(WMGetButtonSelected(panel-alwChk) != 0) ? Yes : No);
+ value1 = WMCreatePLString(buf1);
+ different |= insertAttribute(dict, winDic,
AAlwaysUserIcon, value1, flags);
+ WMReleasePropList(value1);
+ wfree(buf1);
+ }
+ wfree(icon_file);
}
- wfree(icon_file);
}
i = WMGetPopUpButtonSelectedItem(panel-wsP) - 1;
@@ -496,9 +509,6 @@ static void saveSettings(WMButton *button, InspectorPanel
*panel)
flags |= IS_BOOLEAN;
- value = (WMGetButtonSelected(panel-alwChk) != 0) ? Yes : No;
- different |= insertAttribute(dict, winDic, AAlwaysUserIcon, value,
flags);
-
value = (WMGetButtonSelected(panel-attrChk[0]) != 0) ? Yes : No;
different |= insertAttribute(dict, winDic, ANoTitlebar, value, flags);
--
1.7.10.4
From
