Sébastien Guimmara [off-list ref] writes:
Teach generate-cmdlist.sh to parse common command groups
found in command-list.txt in the form
common-3_worktree ('3_worktree' being the group identifier)
Extract the $grp variable, in addition to the previous $cmd,
and inject it as a third field in the cmdname_help struct:
{"add", N_("Add file contents to the index"), "3_worktree"},
So that when 'git' is called, we can display common commands
grouped by theme instead of a less useful alphabetical order.
Reviewed by: Luke Diamand [off-list ref]
Reviewed by: Andreas Schwab [off-list ref]
These people may have helped you to polish your earlier round to
come up with this version, but I do not think they should be listed
as reviewed-by (in the sense that they would say "Yes, I read this
version and consider it very good--I endorse the change!") yet.
Signed-off-by: Sébastien Guimmara <redacted>
---
quoted hunk
generate-cmdlist.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 9a4c9b9..98f937b 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -4,19 +4,20 @@ echo "/* Automatically generated by $0 */
struct cmdname_help {
char name[16];
char help[80];
+ char group[20];
Storing group name in duplicated text for all commands and then
using string comparison at runtime is grossly wasteful, don't you
think, especially when the list is not manually maintained?
command-list.txt is something manually maintained, i.e. the source,
and it makes sense to use easy-to-see strings like "3_worktree"
label there instead of cryptic number, but the whole point of this
script is to pre-process that text into .c file to a form that is
more suited for machine consumption. I think this script should:
- read command list once to make the list of groups in a new
separate array;
- add the group as an "unsigned char group_number" (as we won't
have more than 200 groups) field to this struct;
- read command list again and for each command give the group
number here.
or something like that.
On Sun, May 3, 2015 at 1:55 PM, Junio C Hamano [off-list ref] wrote:
Sébastien Guimmara [off-list ref] writes:
quoted
Teach generate-cmdlist.sh to parse common command groups
found in command-list.txt in the form
common-3_worktree ('3_worktree' being the group identifier)
Extract the $grp variable, in addition to the previous $cmd,
and inject it as a third field in the cmdname_help struct:
{"add", N_("Add file contents to the index"), "3_worktree"},
So that when 'git' is called, we can display common commands
grouped by theme instead of a less useful alphabetical order.
Reviewed by: Luke Diamand [off-list ref]
Reviewed by: Andreas Schwab [off-list ref]
These people may have helped you to polish your earlier round to
come up with this version, but I do not think they should be listed
as reviewed-by (in the sense that they would say "Yes, I read this
version and consider it very good--I endorse the change!") yet.
Minor addendum: If you'd like to acknowledge Luke for $(...) and
Andreas for IFS munging, then Helped-by: would be appropriate (and
doesn't require their consent).
More below.
quoted
Signed-off-by: Sébastien Guimmara <redacted>
---
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 9a4c9b9..98f937b 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -4,19 +4,20 @@ echo "/* Automatically generated by $0 */
struct cmdname_help {
char name[16];
char help[80];
+ char group[20];
Storing group name in duplicated text for all commands and then
using string comparison at runtime is grossly wasteful, don't you
think, especially when the list is not manually maintained?
command-list.txt is something manually maintained, i.e. the source,
and it makes sense to use easy-to-see strings like "3_worktree"
label there instead of cryptic number, but the whole point of this
script is to pre-process that text into .c file to a form that is
more suited for machine consumption. I think this script should:
- read command list once to make the list of groups in a new
separate array;
- add the group as an "unsigned char group_number" (as we won't
have more than 200 groups) field to this struct;
- read command list again and for each command give the group
number here.
or something like that.
Some additional observations:
The "common-N_group" form seems both redundant and unsightly.
Redundant because you could just as easily pluck out 'common' lines by
searching for "N_group", thus making the "common-" prefix unnecessary.
Unsightly because the mix of "-" and "_" is hard to read. (In general,
"-" is easier to read, thus probably a better choice.)
Thinking out loud, without regard to implementation complexity...
I might expect to see the table of groups to be declared first, in the
order that they should be displayed by "git help". Having the groups
in their own table also allows you to give them human-readable
descriptions. For actual use, you'd tag each command with the bare
group name.
So, something like this, perhaps:
[groups]
init starting a working area
info examining history and state
...
branching branching and merging histories
[commands]
git-branch mainporcelain branching
...
git-clone mainporcelain init
...
This way, the 'N' in "N_group" is unnecessary since presentation order
is implied by the [groups] table, and you don't need the "common-"
prefix anymore since any command having an attribute from the [groups]
table is automatically considered common.
On 05/03/2015 10:40 PM, Eric Sunshine wrote:
quoted
These people may have helped you to polish your earlier round to
come up with this version, but I do not think they should be listed
as reviewed-by (in the sense that they would say "Yes, I read this
version and consider it very good--I endorse the change!") yet.
Minor addendum: If you'd like to acknowledge Luke for $(...) and
Andreas for IFS munging, then Helped-by: would be appropriate (and
doesn't require their consent).
Thanks for the precision, I got confused with this one.
So, something like this, perhaps:
[groups]
init starting a working area
info examining history and state
...
branching branching and merging histories
[commands]
git-branch mainporcelain branching
...
git-clone mainporcelain init
...
This way, the 'N' in "N_group" is unnecessary since presentation order
is implied by the [groups] table, and you don't need the "common-"
prefix anymore since any command having an attribute from the [groups]
table is automatically considered common.
It's a good idea. I'll look into it.
On Sun, May 3, 2015 at 4:53 PM, Sébastien Guimmara
[off-list ref] wrote:
On 05/03/2015 10:40 PM, Eric Sunshine wrote:
quoted
quoted
These people may have helped you to polish your earlier round to
come up with this version, but I do not think they should be listed
as reviewed-by (in the sense that they would say "Yes, I read this
version and consider it very good--I endorse the change!") yet.
Minor addendum: If you'd like to acknowledge Luke for $(...) and
Andreas for IFS munging, then Helped-by: would be appropriate (and
doesn't require their consent).
Thanks for the precision, I got confused with this one.
What Junio meant was that Reviewed-by: is something that reviewers
"give" explicitly when they are confident that they understand and
believe your patches are correct. You may incorporate a "given"
Reviewed-by: into a patch when re-rolling if the patch hasn't changed
since the previous round (or has changed in very minor ways which
don't affect the Reviewed-by:). Someone merely reading your patch and
commenting on it (and possibly making suggestions) does not warrant a
Reviewed-by: since comments and suggestions alone don't necessarily
mean that they agree that a patch is acceptable or correct, thus don't
add Reviewed-by: without it being "given".
Helped-by: is for acknowledging someone's assistance.
More below.
quoted
So, something like this, perhaps:
[groups]
init starting a working area
info examining history and state
...
branching branching and merging histories
[commands]
git-branch mainporcelain branching
...
git-clone mainporcelain init
...
This way, the 'N' in "N_group" is unnecessary since presentation order
is implied by the [groups] table, and you don't need the "common-"
prefix anymore since any command having an attribute from the [groups]
table is automatically considered common.
It's a good idea. I'll look into it.
'awk' might be helpful to implement this sort of scheme, as well as
Junio's suggestion of emitting only a group number in the generated
table rather than a string. If so, try to stick to POSIX (and avoid
GNU or other extensions).