Mail Archives: cygwin/2007/09/06/15:25:44
--------------000105000800090107080106
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Hello-
Firstly, thanks to everyone who has worked on setup.exe, it's really a
very convenient program! There is just one thing that has always
bothered me, which is that you have to click repeatedly on the package
or category to cycle through all the available actions to find the one
you want. The main problem is that each click causes the dependencies to
be recalculated, which can cause annoying slowdowns if you're trying to
do something like uninstall all packages in a large category. There is
also the following situation which occurs often, especially when you are
playing around with installing and uninstalling new packages:
-package A requires package B
-package A has two available versions
-package B appears before package A in the list
Now suppose A and B are both installed, and you want to uninstall them.
Since B appears first, you click through to uninstall, no problem. Now
you scroll down, maybe several pages away, and try to uninstall package
A. The first time you click, though, you end up on the Prev version,
which then calculates that it needs package A and goes back and sets
package A to Install again. The only way to uninstall both of them is to
uninstall B first, and then A. When there are multiple dependencies
involved, it can quickly get impossible to get setup to do what you want.
The simplest way I could think of to correct this would be to change the
behavior so that when you click on a Category or a Package, instead of
simply cycling through, you get a little popup menu that asks you what
you want to do instead. This way, you can go directly to Uninstall
without dealing with the intervening options. This also lets you see all
available versions at once, and avoids calculating dependencies
unnecessarily.
I wrote a simple patch that implements this suggestion. Attached are the
outputs of cvs diff (in diff.txt) and cvs diff -n (in diff_n.txt). (I'm
sorry, I don't know much about CVS, is this the preferred way to submit
a patch?). Here is a summary of the changes:
-Created new class PopupMenu in PopupMenu.{h,cc}, which makes a popup
menu at the mouse cursor location and returns the selected item.
-Added #define to resource.h for use by PopupMenu. For now, it just
reserves 100 IDs, supporting arbitrary popup menus with up to 100
entries. (The number 100 is easily configurable in resource.h.)
-Modified PickCategoryLine to open the menu instead of cycling.
-Added new function select_action() to the packagemeta class, which
implements the menu selection. For now, this is done in an extremely
quick and dirty way that simple calls set_action() repeatedly to figure
out which options would have been cycled through. I would be willing to
re-do this in a more efficient way if this patch is deemed useful, but I
don't even think that's necessary, I think it's fine to do it this way
which reuses the already bug-tested code in set_action().
-Modified PickPackageLine to call select_action() instead of
set_action() when the line is clicked.
-Made some minor changes to packagemeta::_action to expose the category
strings as part of the public interface, so they could be reused in the
popup menu.
Anyway I hope this is useful, if this patch isn't acceptable please let
me know and I can fix it or change it. I wasn't sure about conventions
with tabs, line endings, line lengths, etc., for one thing. In general,
I think the problem I have described requires fixing. If you don't think
this solution is an improvement, I can look into fixing it a different
way also.
-Lewis
--------------000105000800090107080106
Content-Type: text/plain;
name="diff.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="diff.txt"
? setup/.deps
? setup/.libs
? setup/Makefile
? setup/PopupMenu.cc
? setup/PopupMenu.h
? setup/config.cache
? setup/config.log
? setup/config.status
? setup/inilex.cc
? setup/iniparse.cc
? setup/iniparse.h
? setup/libtool
? setup/setup_version.c
? setup/csu_util/.deps
? setup/csu_util/.dirstamp
? setup/libgetopt++/.libs
? setup/libgetopt++/Makefile
? setup/libgetopt++/config.log
? setup/libgetopt++/config.status
? setup/libgetopt++/libgetopt++.la
? setup/libgetopt++/libtool
? setup/libgetopt++/include/autoconf.h
? setup/libgetopt++/include/stamp-h1
? setup/libgetopt++/src/.deps
? setup/libgetopt++/src/.dirstamp
? setup/libgetopt++/src/BoolOption.lo
? setup/libgetopt++/src/GetOption.lo
? setup/libgetopt++/src/Option.lo
? setup/libgetopt++/src/OptionSet.lo
? setup/libgetopt++/src/StringOption.lo
? setup/libgetopt++/tests/.deps
? setup/libmd5-rfc/.deps
? setup/libmd5-rfc/.dirstamp
? setup/tests/.deps
? setup/tests/Makefile
Index: setup/Makefile.am
===================================================================
RCS file: /cvs/cygwin-apps/setup/Makefile.am,v
retrieving revision 2.68
diff -r2.68 Makefile.am
230a231,232
> PopupMenu.h \
> PopupMenu.cc \
Index: setup/PickCategoryLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickCategoryLine.cc,v
retrieving revision 2.10
diff -r2.10 PickCategoryLine.cc
18a19
> #include "PopupMenu.h"
113,117c114,120
< ++current_default;
<
< packagedb().markUnVisited();
<
< return set_action (current_default);
---
>
> //create a popup menu for the user to select the desired action
> PopupMenu menu(packagemeta::_actions::captions, packagemeta::_actions::num_actions);
> int const selection = menu.get_selection();
> if(selection<0) return 0;
> packagedb().markUnVisited();
> return set_action(selection);
Index: setup/PickPackageLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickPackageLine.cc,v
retrieving revision 2.21
diff -r2.21 PickPackageLine.cc
137c137,140
< pkg.set_action (pkg.trustp(theView.deftrust));
---
> {
> //pkg.set_action (pkg.trustp(theView.deftrust));
> pkg.select_action(pkg.trustp(theView.deftrust));
> }
Index: setup/package_meta.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.cc,v
retrieving revision 2.52
diff -r2.52 package_meta.cc
20a21
> #include "PopupMenu.h"
68,84c69,76
< char const *
< packagemeta::_actions::caption ()
< {
< switch (_value)
< {
< case 0:
< return "Default";
< case 1:
< return "Install";
< case 2:
< return "Reinstall";
< case 3:
< return "Uninstall";
< }
< // Pacify GCC: (all case options are checked above)
< return 0;
< }
---
> //action caption definitions
> //TODO: this could be grabbed from the resource file instead
> char const* const packagemeta::_actions::captions[packagemeta::_actions::num_actions] = {
> "Default",
> "Install",
> "Reinstall",
> "Uninstall"
> };
102c94
< if (_value > 3)
---
> if (_value >= num_actions)
412a405,428
> void packagemeta::select_action(packageversion const& default_version) {
> vector<string> actions;
>
> //in order to avoid duplicating all of the logic in set_action() above, we will instead just call set_action repeatedly
> //to see what it would have done, and then put all of that in the menu.
> //this is a little silly, but the assumption is that set_action() is fast, so it isn't such a waste to just call it once for
> //all actions, and then call it again for the one the user actually wanted.
> //this is quick and dirty, a better solution would be to get rid of set_action, and just reformulate its logic into this function.
> //I am willing to do that, if there is agreement that this is an improvement over the old way of just cycling through. -Lewis
>
> string next_action=action_caption();
> do {
> actions.push_back(next_action);
> set_action(default_version);
> next_action = action_caption();
> } while(next_action != actions.front());
>
> PopupMenu menu(actions);
> int const selection = menu.get_selection();
>
> //again we use a silly inefficient for loop so we don't have to think about what all set_action was actually doing.
> for(int i=0; i<selection; ++i) set_action(default_version); //note case i<0 (no selection) is handled correctly.
> }
>
Index: setup/package_meta.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.h,v
retrieving revision 2.36
diff -r2.36 package_meta.h
62a63,64
> enum {num_actions = 4};
> static char const* const captions[num_actions];
65,66c67,68
< _value = aInt;
< if (_value < 0 || _value > 3)
---
> _value = aInt;
> if (_value < 0 || _value >= num_actions)
72c74,75
< const char *caption ();
---
> const char *caption() const {return caption(_value);}
> static const char* caption(int const value) {return captions[value];}
80a84
> void select_action(packageversion const &default_version);
Index: setup/resource.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/resource.h,v
retrieving revision 2.35
diff -r2.35 resource.h
161a162,167
>
> //popup menus
>
> //note: numbers 600-699 are reserved for popup menus; see PopupMenu.h
> #define IDM_POPUP_FIRST 600
> #define IDM_POPUP_LAST 699
--------------000105000800090107080106
Content-Type: text/plain;
name="diff_n.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="diff_n.txt"
? setup/.deps
? setup/.libs
? setup/Makefile
? setup/PopupMenu.cc
? setup/PopupMenu.h
? setup/config.cache
? setup/config.log
? setup/config.status
? setup/inilex.cc
? setup/iniparse.cc
? setup/iniparse.h
? setup/libtool
? setup/setup_version.c
? setup/csu_util/.deps
? setup/csu_util/.dirstamp
? setup/libgetopt++/.libs
? setup/libgetopt++/Makefile
? setup/libgetopt++/config.log
? setup/libgetopt++/config.status
? setup/libgetopt++/libgetopt++.la
? setup/libgetopt++/libtool
? setup/libgetopt++/include/autoconf.h
? setup/libgetopt++/include/stamp-h1
? setup/libgetopt++/src/.deps
? setup/libgetopt++/src/.dirstamp
? setup/libgetopt++/src/BoolOption.lo
? setup/libgetopt++/src/GetOption.lo
? setup/libgetopt++/src/Option.lo
? setup/libgetopt++/src/OptionSet.lo
? setup/libgetopt++/src/StringOption.lo
? setup/libgetopt++/tests/.deps
? setup/libmd5-rfc/.deps
? setup/libmd5-rfc/.dirstamp
? setup/tests/.deps
? setup/tests/Makefile
Index: setup/Makefile.am
===================================================================
RCS file: /cvs/cygwin-apps/setup/Makefile.am,v
retrieving revision 2.68
diff -n -r2.68 Makefile.am
a230 2
PopupMenu.h \
PopupMenu.cc \
Index: setup/PickCategoryLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickCategoryLine.cc,v
retrieving revision 2.10
diff -n -r2.10 PickCategoryLine.cc
a18 1
#include "PopupMenu.h"
d113 5
a117 7
//create a popup menu for the user to select the desired action
PopupMenu menu(packagemeta::_actions::captions, packagemeta::_actions::num_actions);
int const selection = menu.get_selection();
if(selection<0) return 0;
packagedb().markUnVisited();
return set_action(selection);
Index: setup/PickPackageLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickPackageLine.cc,v
retrieving revision 2.21
diff -n -r2.21 PickPackageLine.cc
d137 1
a137 4
{
//pkg.set_action (pkg.trustp(theView.deftrust));
pkg.select_action(pkg.trustp(theView.deftrust));
}
Index: setup/package_meta.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.cc,v
retrieving revision 2.52
diff -n -r2.52 package_meta.cc
a20 1
#include "PopupMenu.h"
d68 17
a84 8
//action caption definitions
//TODO: this could be grabbed from the resource file instead
char const* const packagemeta::_actions::captions[packagemeta::_actions::num_actions] = {
"Default",
"Install",
"Reinstall",
"Uninstall"
};
d102 1
a102 1
if (_value >= num_actions)
a412 24
void packagemeta::select_action(packageversion const& default_version) {
vector<string> actions;
//in order to avoid duplicating all of the logic in set_action() above, we will instead just call set_action repeatedly
//to see what it would have done, and then put all of that in the menu.
//this is a little silly, but the assumption is that set_action() is fast, so it isn't such a waste to just call it once for
//all actions, and then call it again for the one the user actually wanted.
//this is quick and dirty, a better solution would be to get rid of set_action, and just reformulate its logic into this function.
//I am willing to do that, if there is agreement that this is an improvement over the old way of just cycling through. -Lewis
string next_action=action_caption();
do {
actions.push_back(next_action);
set_action(default_version);
next_action = action_caption();
} while(next_action != actions.front());
PopupMenu menu(actions);
int const selection = menu.get_selection();
//again we use a silly inefficient for loop so we don't have to think about what all set_action was actually doing.
for(int i=0; i<selection; ++i) set_action(default_version); //note case i<0 (no selection) is handled correctly.
}
Index: setup/package_meta.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.h,v
retrieving revision 2.36
diff -n -r2.36 package_meta.h
a62 2
enum {num_actions = 4};
static char const* const captions[num_actions];
d65 2
a66 2
_value = aInt;
if (_value < 0 || _value >= num_actions)
d72 1
a72 2
const char *caption() const {return caption(_value);}
static const char* caption(int const value) {return captions[value];}
a80 1
void select_action(packageversion const &default_version);
Index: setup/resource.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/resource.h,v
retrieving revision 2.35
diff -n -r2.35 resource.h
a161 6
//popup menus
//note: numbers 600-699 are reserved for popup menus; see PopupMenu.h
#define IDM_POPUP_FIRST 600
#define IDM_POPUP_LAST 699
--------------000105000800090107080106
Content-Type: text/plain; charset=us-ascii
--
Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple
Problem reports: http://cygwin.com/problems.html
Documentation: http://cygwin.com/docs.html
FAQ: http://cygwin.com/faq/
--------------000105000800090107080106--
- Raw text -