Re: setup.exe suggestion + patch
Lewis Hyatt wrote: 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: Did anyone happen to take a look at this message I sent last week? I thought maybe it would be interesting to someone besides me, but maybe I'm wrong. If you don't like the extra clicks involved in my solution, I would be willing to explore others as well. I do think the problem I mentioned in my original message is worth fixing... -Lewis -- 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/
RE: setup.exe suggestion + patch
On 13 September 2007 18:57, Lewis Hyatt wrote: Lewis Hyatt wrote: 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: Did anyone happen to take a look at this message I sent last week? Not yet, but I plan to. (Limited free time at the moment). I thought maybe it would be interesting to someone besides me, but maybe I'm wrong. Nooo, you're dead right. Fixing this has been long overdue, thanks for your effort and sorry for not having got on the case sooner! cheers, DaveK -- Can't think of a witty .sigline today -- 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/
Re: setup.exe suggestion + patch
On Thu, 6 Sep 2007, Lewis Hyatt wrote:
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.
First off, thank you for the popup menu implementation -- I was planning
to use it for something else in setup, and now I don't have to write it.
Second, I agree that the problem is good, and I like your solution of
selection vs. cycling.
Third, I have to apologize -- I've had a partial reply to your message
sitting in my drafts since the day you sent it, but got bogged down.
A few comments on the patch:
1) It would be great if you used diff -up -- unified diffs are so much
easier to read.
2) Is there a reason you use popup menus, rather than pull-down lists?
3) [Minor] You'd use the GNU coding guidelines for whitespace and
indentation.
If you could resend a unified diff, I'll apply it in my tree and test it
out.
Igor
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_ [EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`'-. ;-;;,_Igor Peshansky, Ph.D. (name changed!)
|,4- ) )-,_. ,\ ( `'-'old name: Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
Belief can be manipulated. Only knowledge is dangerous. -- Frank Herbert
--
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/
Re: setup.exe suggestion + patch
Igor Peshansky wrote:
Third, I have to apologize -- I've had a partial reply to your message
sitting in my drafts since the day you sent it, but got bogged down.
OK, I understand of course, I just wanted to check. Thanks for getting
back to me Igor and Dave!
A few comments on the patch:
1) It would be great if you used diff -up -- unified diffs are so much
easier to read.
I attached the output of cvs diff -up. I also attached the two new files
PopupMenu.{cc,h}, which I didn't realize were not included in the diff
already.
2) Is there a reason you use popup menus, rather than pull-down lists?
I think pull-down lists would probably be nicer and more intuitive.
Adding this is a much larger change, though, because you have to change
the PickLine classes to paint the controls, and you have to modify the
window procedures, etc. I can do all that, but wanted to gauge interest
first. The drop-down menu way only takes a couple lines, and doesn't
require dealing with the window procedure, so I thought it was a good
way to start anyway. Note that my implementation of
packagemeta::select_action(), which builds the menu, is also admittedly
pretty quick and dirty. It's clear how to improve it, but it works fine
the way it is and re-uses the already-debugged packagemeta::set_action()
code.
-Lewis
/*
* Copyright (c) 2002 Lewis Hyatt.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* A copy of the GNU General Public License can be found at
* http://www.gnu.org/
*
* Written by Lewis Hyatt [EMAIL PROTECTED]
*
*/
#include PopupMenu.h
#include Exception.h
#include resource.h
#include String++.h
using namespace std;
void PopupMenu::init_menu(char const* const* const items, size_t const
num_items)
{
hMenu=0;
static size_t const max_items = IDM_POPUP_LAST - IDM_POPUP_FIRST + 1;
if(num_items max_items) throw new Exception(TOSTRING (__LINE__)
__FILE__, Too many popup menu items);
hMenu = CreatePopupMenu();
for(size_t i=0; i!=num_items; ++i)
AppendMenu(hMenu, MF_STRING, IDM_POPUP_FIRST+i, items[i]);
}
void PopupMenu::init_menu(string const* const items, size_t const num_items)
{
vectorchar const* items_c(num_items);
for(size_t i=0; i!=num_items; ++i) items_c[i] = items[i].c_str();
init_menu(items_c[0], num_items);
}
int PopupMenu::get_selection(int x, int y) const {
if(x==position_undefined || y==position_undefined) {
POINT point;
GetCursorPos(point);
if(x==position_undefined) x=point.x;
if(y==position_undefined) y=point.y;
}
int const result = TrackPopupMenuEx(
hMenu,
TPM_CENTERALIGN | TPM_TOPALIGN | TPM_NONOTIFY | TPM_RETURNCMD,
x, y,
GetActiveWindow(),
0
);
return result==0 ? -1 : result - IDM_POPUP_FIRST;
}
/*
* Copyright (c) 2007 Lewis Hyatt.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* A copy of the GNU General Public License can be found at
* http://www.gnu.org/
*
* Written by Lewis Hyatt [EMAIL PROTECTED]
*
*/
//the purpose of this class is to create a popup menu on the fly from a list of
strings.
//the menu is displayed and then the index of the selected string is returned,
or -1 if
//no string was selected. this class needs a block of IDs reserved for the menu
items,
//which is defined by IDM_POPUP_FIRST and IDM_POPUP_LAST in resource.h.
#ifndef SETUP_POPUPMENU_H
#define SETUP_POPUPMENU_H
#include win32.h
#include string
#include vector
class PopupMenu {
HMENU hMenu;
//make non-copyable for simplicity
PopupMenu(PopupMenu const);
PopupMenu operator=(PopupMenu const*);
//the init function is private because it is only called from the
constructor
void init_menu(char const* const* items, size_t num_items);
void init_menu(std::string const* items, size_t num_items);
public:
//constructor creates the menu but does not show it. there are a few
overloads so you can pass an
//array of char*, an array of string, or a vector of string.
PopupMenu(char const* const* items, size_t num_items)
{
init_menu(items, num_items);
}
PopupMenu(std::string const* items, size_t num_items)
{
init_menu(items, num_items);
}
setup.exe suggestion + patch
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
? 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
