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)
{
vector<char 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);
}
explicit PopupMenu(std::vector<std::string> const& items)
{
init_menu(&items[0], items.size());
}
//get_selection shows the menu and returns the index of the selected
item.
//the x and y parameters let you pick where to show the menu; leave them
//undefined to show at the current cursor location.
enum {position_undefined = -1};
int get_selection(int x=position_undefined, int y=position_undefined)
const;
//destructor frees the menu
~PopupMenu()
{
DestroyMenu(hMenu);
}
};
#endif /* SETUP_POPUPMENU_H */
? 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 -u -p -r2.68 Makefile.am
--- setup/Makefile.am 30 Jul 2007 22:55:49 -0000 2.68
+++ setup/Makefile.am 13 Sep 2007 18:37:32 -0000
@@ -228,6 +228,8 @@ setup_SOURCES = \
PickPackageLine.h \
PickView.cc \
PickView.h \
+ PopupMenu.h \
+ PopupMenu.cc \
postinstall.cc \
prereq.cc \
prereq.h \
Index: setup/PickCategoryLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickCategoryLine.cc,v
retrieving revision 2.10
diff -u -p -r2.10 PickCategoryLine.cc
--- setup/PickCategoryLine.cc 24 May 2006 13:01:34 -0000 2.10
+++ setup/PickCategoryLine.cc 13 Sep 2007 18:37:32 -0000
@@ -16,6 +16,7 @@
#include "PickCategoryLine.h"
#include "package_db.h"
#include "PickView.h"
+#include "PopupMenu.h"
void
PickCategoryLine::empty (void)
@@ -110,11 +111,12 @@ PickCategoryLine::click (int const myrow
{
if ((size_t) x >= spin_x)
{
- ++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);
}
else
{
Index: setup/PickPackageLine.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/PickPackageLine.cc,v
retrieving revision 2.21
diff -u -p -r2.21 PickPackageLine.cc
--- setup/PickPackageLine.cc 24 May 2006 13:01:34 -0000 2.21
+++ setup/PickPackageLine.cc 13 Sep 2007 18:37:32 -0000
@@ -134,7 +134,10 @@ PickPackageLine::click (int const myrow,
if (x >= theView.headers[theView.new_col].x - HMARGIN / 2
&& x <= theView.headers[theView.new_col + 1].x - HMARGIN / 2)
- pkg.set_action (pkg.trustp(theView.deftrust));
+ {
+ //pkg.set_action (pkg.trustp(theView.deftrust));
+ pkg.select_action(pkg.trustp(theView.deftrust));
+ }
packagedb().markUnVisited();
/* Add any packages that are needed by this package */
Index: setup/package_meta.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.cc,v
retrieving revision 2.52
diff -u -p -r2.52 package_meta.cc
--- setup/package_meta.cc 17 Apr 2006 16:13:17 -0000 2.52
+++ setup/package_meta.cc 13 Sep 2007 18:37:33 -0000
@@ -18,6 +18,7 @@ static const char *cvsid = "\n%%% $Id: p
#endif
#include "package_meta.h"
+#include "PopupMenu.h"
#include <string>
#include <set>
@@ -65,23 +66,14 @@ const
packagemeta::_actions
packagemeta::Uninstall_action (3);
-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"
+};
packagemeta::packagemeta (packagemeta const &rhs) :
name (rhs.name), key (rhs.name), installed_from (),
@@ -99,7 +91,7 @@ packagemeta::packagemeta (packagemeta co
packagemeta::_actions & packagemeta::_actions::operator++ ()
{
++_value;
- if (_value > 3)
+ if (_value >= num_actions)
_value = 0;
return *this;
}
@@ -410,6 +402,30 @@ packagemeta::set_action (packageversion
}
}
+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.
+}
+
int
packagemeta::set_requirements (trusts deftrust, size_t depth)
{
Index: setup/package_meta.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.h,v
retrieving revision 2.36
diff -u -p -r2.36 package_meta.h
--- setup/package_meta.h 17 Apr 2006 16:13:17 -0000 2.36
+++ setup/package_meta.h 13 Sep 2007 18:37:33 -0000
@@ -60,16 +60,19 @@ public:
class _actions
{
public:
+ enum {num_actions = 4};
+ static char const* const captions[num_actions];
_actions ():_value (0) {};
_actions (int aInt) {
- _value = aInt;
- if (_value < 0 || _value > 3)
+ _value = aInt;
+ if (_value < 0 || _value >= num_actions)
_value = 0;
}
_actions & operator ++ ();
bool operator == (_actions const &rhs) { return _value == rhs._value; }
bool operator != (_actions const &rhs) { return _value != rhs._value; }
- const char *caption ();
+ const char *caption() const {return caption(_value);}
+ static const char* caption(int const value) {return captions[value];}
private:
int _value;
};
@@ -78,6 +81,7 @@ public:
static const _actions Reinstall_action;
static const _actions Uninstall_action;
void set_action (packageversion const &default_version);
+ void select_action(packageversion const &default_version);
void set_action (_actions, packageversion const & default_version);
void uninstall ();
int set_requirements (trusts deftrust, size_t depth);
Index: setup/resource.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/resource.h,v
retrieving revision 2.35
diff -u -p -r2.35 resource.h
--- setup/resource.h 4 May 2007 21:56:53 -0000 2.35
+++ setup/resource.h 13 Sep 2007 18:37:35 -0000
@@ -159,3 +159,9 @@
#define IDC_STATUS_HEADER 582
#define IDC_STATUS 583
#define IDC_STATIC_HEADER 584
+
+//popup menus
+
+//note: numbers 600-699 are reserved for popup menus; see PopupMenu.h
+#define IDM_POPUP_FIRST 600
+#define IDM_POPUP_LAST 699
--
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/
