| Message ID | 2e308393edb0c34593e78603b43bee8d3a45dd8a.1715339393.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | builtin/config: remove global state | expand |
On Fri, May 10, 2024 at 4:26 AM Patrick Steinhardt <ps@pks.im> wrote: > > We only use actions in the legacy mode. Convert them to an enum and move > them into `cmd_config_actions()` to clearly demonstrate that they are > not used anywhere else. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > builtin/config.c | 44 +++++++++++++++++++------------------------- > 1 file changed, 19 insertions(+), 25 deletions(-) > > diff --git a/builtin/config.c b/builtin/config.c > index e9956574fe..8f3342b593 100644 > --- a/builtin/config.c > +++ b/builtin/config.c > @@ -87,30 +87,6 @@ static int show_origin; > static int show_scope; > static int fixed_value; > > -#define ACTION_GET (1<<0) > -#define ACTION_GET_ALL (1<<1) > -#define ACTION_GET_REGEXP (1<<2) > -#define ACTION_REPLACE_ALL (1<<3) > -#define ACTION_ADD (1<<4) > -#define ACTION_UNSET (1<<5) > -#define ACTION_UNSET_ALL (1<<6) > -#define ACTION_RENAME_SECTION (1<<7) > -#define ACTION_REMOVE_SECTION (1<<8) > -#define ACTION_LIST (1<<9) > -#define ACTION_EDIT (1<<10) > -#define ACTION_SET (1<<11) > -#define ACTION_SET_ALL (1<<12) > -#define ACTION_GET_COLOR (1<<13) > -#define ACTION_GET_COLORBOOL (1<<14) > -#define ACTION_GET_URLMATCH (1<<15) > - > -/* > - * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than > - * one line of output and which should therefore be paged. > - */ We lost this comment when inlining this. Not sure if that was intentional (comment deemed to be of low value) or accidental. I think as someone unfamiliar with this logic that the comment would be helpful to keep - it describes _why_ those items are checked for (vs. needing to infer it from the action that's taken when there's a match). > -#define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \ > - ACTION_GET_REGEXP | ACTION_GET_URLMATCH) > - > #define TYPE_BOOL 1 > #define TYPE_INT 2 > #define TYPE_BOOL_OR_INT 3 > @@ -1031,6 +1007,24 @@ static int cmd_config_edit(int argc, const char **argv, const char *prefix) > > static int cmd_config_actions(int argc, const char **argv, const char *prefix) > { > + enum { > + ACTION_GET = (1<<0), > + ACTION_GET_ALL = (1<<1), > + ACTION_GET_REGEXP = (1<<2), > + ACTION_REPLACE_ALL = (1<<3), > + ACTION_ADD = (1<<4), > + ACTION_UNSET = (1<<5), > + ACTION_UNSET_ALL = (1<<6), > + ACTION_RENAME_SECTION = (1<<7), > + ACTION_REMOVE_SECTION = (1<<8), > + ACTION_LIST = (1<<9), > + ACTION_EDIT = (1<<10), > + ACTION_SET = (1<<11), > + ACTION_SET_ALL = (1<<12), > + ACTION_GET_COLOR = (1<<13), > + ACTION_GET_COLORBOOL = (1<<14), > + ACTION_GET_URLMATCH = (1<<15), > + }; > const char *comment_arg = NULL; > int actions = 0; > struct option opts[] = { > @@ -1147,7 +1141,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix) > > comment = git_config_prepare_comment_string(comment_arg); > > - if (actions & PAGING_ACTIONS) > + if (actions & (ACTION_LIST | ACTION_GET_ALL | ACTION_GET_REGEXP | ACTION_GET_URLMATCH)) > setup_auto_pager("config", 1); > > if (actions == ACTION_LIST) { > -- > 2.45.GIT >
On Fri, May 10, 2024 at 01:42:49PM -0700, Kyle Lippincott wrote: > On Fri, May 10, 2024 at 4:26 AM Patrick Steinhardt <ps@pks.im> wrote: [snip] > > -/* > > - * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than > > - * one line of output and which should therefore be paged. > > - */ > > We lost this comment when inlining this. Not sure if that was > intentional (comment deemed to be of low value) or accidental. I think > as someone unfamiliar with this logic that the comment would be > helpful to keep - it describes _why_ those items are checked for (vs. > needing to infer it from the action that's taken when there's a > match). It was intentional because I found the comment to be somewhat redundant. But I can also see that it does provide a bit more context than the code itself does, so I'll add it back in. Patrick
diff --git a/builtin/config.c b/builtin/config.c index e9956574fe..8f3342b593 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -87,30 +87,6 @@ static int show_origin; static int show_scope; static int fixed_value; -#define ACTION_GET (1<<0) -#define ACTION_GET_ALL (1<<1) -#define ACTION_GET_REGEXP (1<<2) -#define ACTION_REPLACE_ALL (1<<3) -#define ACTION_ADD (1<<4) -#define ACTION_UNSET (1<<5) -#define ACTION_UNSET_ALL (1<<6) -#define ACTION_RENAME_SECTION (1<<7) -#define ACTION_REMOVE_SECTION (1<<8) -#define ACTION_LIST (1<<9) -#define ACTION_EDIT (1<<10) -#define ACTION_SET (1<<11) -#define ACTION_SET_ALL (1<<12) -#define ACTION_GET_COLOR (1<<13) -#define ACTION_GET_COLORBOOL (1<<14) -#define ACTION_GET_URLMATCH (1<<15) - -/* - * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than - * one line of output and which should therefore be paged. - */ -#define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \ - ACTION_GET_REGEXP | ACTION_GET_URLMATCH) - #define TYPE_BOOL 1 #define TYPE_INT 2 #define TYPE_BOOL_OR_INT 3 @@ -1031,6 +1007,24 @@ static int cmd_config_edit(int argc, const char **argv, const char *prefix) static int cmd_config_actions(int argc, const char **argv, const char *prefix) { + enum { + ACTION_GET = (1<<0), + ACTION_GET_ALL = (1<<1), + ACTION_GET_REGEXP = (1<<2), + ACTION_REPLACE_ALL = (1<<3), + ACTION_ADD = (1<<4), + ACTION_UNSET = (1<<5), + ACTION_UNSET_ALL = (1<<6), + ACTION_RENAME_SECTION = (1<<7), + ACTION_REMOVE_SECTION = (1<<8), + ACTION_LIST = (1<<9), + ACTION_EDIT = (1<<10), + ACTION_SET = (1<<11), + ACTION_SET_ALL = (1<<12), + ACTION_GET_COLOR = (1<<13), + ACTION_GET_COLORBOOL = (1<<14), + ACTION_GET_URLMATCH = (1<<15), + }; const char *comment_arg = NULL; int actions = 0; struct option opts[] = { @@ -1147,7 +1141,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix) comment = git_config_prepare_comment_string(comment_arg); - if (actions & PAGING_ACTIONS) + if (actions & (ACTION_LIST | ACTION_GET_ALL | ACTION_GET_REGEXP | ACTION_GET_URLMATCH)) setup_auto_pager("config", 1); if (actions == ACTION_LIST) {
We only use actions in the legacy mode. Convert them to an enum and move them into `cmd_config_actions()` to clearly demonstrate that they are not used anywhere else. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/config.c | 44 +++++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 25 deletions(-)