[pacman-dev] [PATCH] Sort short options before long options in --help
$ pacman -Uh
options:
-b, --dbpath path set an alternate database location
-d, --nodeps skip dependency checks
-f, --force force install, overwrite conflicting files
-k, --dbonly only modify database entries, not package files
-r, --root pathset an alternate installation root
-v, --verbosebe verbose
--arch archset an alternate architecture
--asdeps install packages as non-explicitly installed
--asexplicit install packages as explicitly installed
--cachedir dir set an alternate package cache location
--config path set an alternate configuration file
--debug display debug messages
--ignore pkg ignore a package upgrade (can be used more than once)
--ignoregroup grp
ignore a group upgrade (can be used more than once)
--logfile path set an alternate log file
--noconfirm do not ask for any confirmation
--noprogressbar do not show a progress bar when downloading files
--noscriptletdo not execute the install scriptlet if one exists
--print only print the targets instead of performing the
operation
--print-format string
specify how the targets should be printed
Signed-off-by: Xavier Chantry chantry.xav...@gmail.com
---
src/pacman/pacman.c | 27 ++-
1 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index 7d51124..3bfe6ef 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -28,6 +28,7 @@
#include stdlib.h /* atoi */
#include stdio.h
+#include ctype.h /* isspace */
#include limits.h
#include getopt.h
#include string.h
@@ -59,6 +60,30 @@ pmdb_t *db_local;
/* list of targets specified on command line */
static alpm_list_t *pm_targets;
+/* Used to sort the options in --help */
+static int options_cmp(const void *p1, const void *p2)
+{
+ const char *s1 = p1;
+ const char *s2 = p2;
+ int ret = 0;
+
+ /* First skip all spaces in both strings */
+ while(isspace((unsigned char)*s1))
+ s1++;
+ while(isspace((unsigned char)*s2))
+ s2++;
+ /* If we compare a long option (--abcd) and a short one (-a),
+* the short one always wins */
+ if (*(s1+1) != '-' *(s2+1) == '-') {
+ ret = -1;
+ } else if (*(s2+1) != '-' *(s1+1) == '-') {
+ ret = 1;
+ } else {
+ ret = strcmp(s1, s2);
+ }
+ return(ret);
+}
+
/** Display usage/syntax for the specified operation.
* @param op the operation code requested
* @param myname basename(argv[0])
@@ -166,7 +191,7 @@ static void usage(int op, const char * const myname)
addlist(_( --logfile path set an alternate log
file\n));
addlist(_( --noconfirm do not ask for any
confirmation\n));
}
- list = alpm_list_msort(list, alpm_list_count(list), str_cmp);
+ list = alpm_list_msort(list, alpm_list_count(list), options_cmp);
for (i = list; i; i = alpm_list_next(i)) {
printf(%s, (char *)alpm_list_getdata(i));
}
--
1.7.3.1
Re: [pacman-dev] [PATCH] Sort short options before long options in --help
$ pacman -Uh
options:
-b, --dbpath path set an alternate database location
-d, --nodeps skip dependency checks
-f, --force force install, overwrite conflicting files
-k, --dbonly only modify database entries, not package files
-r, --root pathset an alternate installation root
-v, --verbosebe verbose
--arch archset an alternate architecture
--asdeps install packages as non-explicitly installed
--asexplicit install packages as explicitly installed
--cachedir dir set an alternate package cache location
--config path set an alternate configuration file
--debug display debug messages
--ignore pkg ignore a package upgrade (can be used more than once)
--ignoregroup grp
ignore a group upgrade (can be used more than once)
--logfile path set an alternate log file
--noconfirm do not ask for any confirmation
--noprogressbar do not show a progress bar when downloading files
--noscriptletdo not execute the install scriptlet if one exists
--print only print the targets instead of performing the
operation
--print-format string
specify how the targets should be printed
Signed-off-by: Xavier Chantry chantry.xav...@gmail.com
I like this.
+/* Used to sort the options in --help */
+static int options_cmp(const void *p1, const void *p2)
+{
+ const char *s1 = p1;
+ const char *s2 = p2;
+ int ret = 0;
+
+ /* First skip all spaces in both strings */
+ while(isspace((unsigned char)*s1))
+ s1++;
+ while(isspace((unsigned char)*s2))
+ s2++;
+ /* If we compare a long option (--abcd) and a short one (-a),
+ * the short one always wins */
+ if (*(s1+1) != '-' *(s2+1) == '-') {
+ ret = -1;
I hope you don't pass invalid string arguments (e.g. empty string),
otherwise we can get a segfault here.
+ } else if (*(s2+1) != '-' *(s1+1) == '-') {
+ ret = 1;
+ } else {
+ ret = strcmp(s1, s2);
+ }
+ return(ret);
+}
NG
Re: [pacman-dev] [PATCH] Sort short options before long options in --help
On Thu, Oct 14, 2010 at 1:55 PM, Nagy Gabor ng...@bibl.u-szeged.hu wrote:
I like this.
Cool :)
+ /* First skip all spaces in both strings */
+ while(isspace((unsigned char)*s1))
+ s1++;
+ while(isspace((unsigned char)*s2))
+ s2++;
+ /* If we compare a long option (--abcd) and a short one (-a),
+ * the short one always wins */
+ if (*(s1+1) != '-' *(s2+1) == '-') {
+ ret = -1;
I hope you don't pass invalid string arguments (e.g. empty string),
otherwise we can get a segfault here.
I missed something very important. I thought we had complete controls
on these strings, while in fact we don't have any control at all,
since it's the gettext-ed strings that I sort :P
I will make a safer version, thanks.
[pacman-dev] [PATCH] Sort short options before long options in --help
$ pacman -Uh
options:
-b, --dbpath path set an alternate database location
-d, --nodeps skip dependency checks
-f, --force force install, overwrite conflicting files
-k, --dbonly only modify database entries, not package files
-r, --root pathset an alternate installation root
-v, --verbosebe verbose
--arch archset an alternate architecture
--asdeps install packages as non-explicitly installed
--asexplicit install packages as explicitly installed
--cachedir dir set an alternate package cache location
--config path set an alternate configuration file
--debug display debug messages
--ignore pkg ignore a package upgrade (can be used more than once)
--ignoregroup grp
ignore a group upgrade (can be used more than once)
--logfile path set an alternate log file
--noconfirm do not ask for any confirmation
--noprogressbar do not show a progress bar when downloading files
--noscriptletdo not execute the install scriptlet if one exists
--print only print the targets instead of performing the
operation
--print-format string
specify how the targets should be printed
Signed-off-by: Xavier Chantry chantry.xav...@gmail.com
---
src/pacman/pacman.c | 39 ++-
1 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index 7d51124..38d7f6a 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -26,8 +26,10 @@
#define PACKAGE_VERSION GIT_VERSION
#endif
+#include assert.h
#include stdlib.h /* atoi */
#include stdio.h
+#include ctype.h /* isspace */
#include limits.h
#include getopt.h
#include string.h
@@ -59,6 +61,41 @@ pmdb_t *db_local;
/* list of targets specified on command line */
static alpm_list_t *pm_targets;
+/* Used to sort the options in --help */
+static int options_cmp(const void *p1, const void *p2)
+{
+ const char *s1 = p1;
+ const char *s2 = p2;
+
+ assert(s1);
+ assert(s2);
+ /* First skip all spaces in both strings */
+ while(isspace((unsigned char)*s1))
+ s1++;
+ while(isspace((unsigned char)*s2))
+ s2++;
+ /* If we compare a long option (--abcd) and a short one (-a),
+* the short one always wins */
+ if(*s1 == '-' *s2 == '-') {
+ s1++;
+ s2++;
+ if(*s1 == '-' *s2 == '-') {
+ /* two long - strcmp */
+ s1++;
+ s2++;
+ } else if(*s2 == '-') {
+ /* s1 short, s2 long */
+ return(-1);
+ } else if(*s1 == '-') {
+ /* s1 long, s2 short */
+ return(1);
+ }
+ /* two short - strcmp */
+ }
+
+ return(strcmp(s1, s2));
+}
+
/** Display usage/syntax for the specified operation.
* @param op the operation code requested
* @param myname basename(argv[0])
@@ -166,7 +203,7 @@ static void usage(int op, const char * const myname)
addlist(_( --logfile path set an alternate log
file\n));
addlist(_( --noconfirm do not ask for any
confirmation\n));
}
- list = alpm_list_msort(list, alpm_list_count(list), str_cmp);
+ list = alpm_list_msort(list, alpm_list_count(list), options_cmp);
for (i = list; i; i = alpm_list_next(i)) {
printf(%s, (char *)alpm_list_getdata(i));
}
--
1.7.3.1
Re: [pacman-dev] [PATCH] Sort short options before long options in --help
On Thu, Oct 14, 2010 at 8:36 AM, Xavier Chantry
chantry.xav...@gmail.com wrote:
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index 7d51124..38d7f6a 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -26,8 +26,10 @@
#define PACKAGE_VERSION GIT_VERSION
#endif
+#include assert.h
#include stdlib.h /* atoi */
#include stdio.h
+#include ctype.h /* isspace */
#include limits.h
#include getopt.h
#include string.h
@@ -59,6 +61,41 @@ pmdb_t *db_local;
/* list of targets specified on command line */
static alpm_list_t *pm_targets;
+/* Used to sort the options in --help */
+static int options_cmp(const void *p1, const void *p2)
+{
+ const char *s1 = p1;
+ const char *s2 = p2;
+
+ assert(s1);
+ assert(s2);
Is the assert stuff you added to be safer? We don't want to use this.
One, they are compiled into nothing if NDEBUG is defined, and two, it
is a bit silly to abort on this condition. cmp(NULL, NULL) ==0, and
then just decide whether to sort NULL first or last.
-Dan
Re: [pacman-dev] [PATCH] Sort short options before long options in --help
On Thu, Oct 14, 2010 at 3:44 PM, Dan McGee dpmc...@gmail.com wrote: Is the assert stuff you added to be safer? We don't want to use this. One, they are compiled into nothing if NDEBUG is defined, and two, it is a bit silly to abort on this condition. cmp(NULL, NULL) ==0, and then just decide whether to sort NULL first or last. It was not supposed to be safer, just to explicitly state that this case should not happen. If gettext returns NULL, our --help output is going to look awesome : (null)(null)(null).. It's not perfectly clear to me that an assert is worse. In the unlikely case that this could be triggered, at least an assert would fail gracefully telling us where it failed and possibly giving a core dump that would tell us more about what happened. Anyway we could debate over and over about assert usage. Maybe it's more interesting for an expensive check in a hot-path that you do not want to keep in a release build, but you want to make sure that the assert is indeed not triggered while developing / testing. I updated the patch with an explicit check (I actually already did this before choosing to use assert) : http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=parseargs
