[RFC,v2,04/13] walken: add handler to git_config
diff mbox series

Message ID 20190626235032.177551-5-emilyshaffer@google.com
State New
Headers show
Series
  • example implementation of revwalk tutorial
Related show

Commit Message

Emily Shaffer June 26, 2019, 11:50 p.m. UTC
For now, we have no configuration options we want to set up for
ourselves, but in the future we may need to. At the very least, we
should invoke git_default_config() for each config option; we will do so
inside of a skeleton config callback so that we know where to add
configuration handling later on when we need it.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 builtin/walken.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

Comments

Eric Sunshine June 27, 2019, 4:54 a.m. UTC | #1
On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> wrote:
> For now, we have no configuration options we want to set up for
> ourselves, but in the future we may need to. At the very least, we
> should invoke git_default_config() for each config option; we will do so
> inside of a skeleton config callback so that we know where to add
> configuration handling later on when we need it.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> diff --git a/builtin/walken.c b/builtin/walken.c
> @@ -24,11 +25,36 @@ const char * const walken_usage[] = {
>  static void init_walken_defaults(void)
>  {
>         /*
> -        * We don't actually need the same components `git log` does; leave this
> -        * empty for now.
> +        * We don't use any other components or have settings to initialize, so
> +        * leave this empty.
>          */
>  }

Meh, I don't think this change has anything to do with this patch. If
the rewritten text is the one you prefer, then just introduce it like
that in patch 3/13 where the function itself was introduced.
Emily Shaffer June 27, 2019, 6:47 p.m. UTC | #2
On Thu, Jun 27, 2019 at 12:54:15AM -0400, Eric Sunshine wrote:
> On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> wrote:
> > For now, we have no configuration options we want to set up for
> > ourselves, but in the future we may need to. At the very least, we
> > should invoke git_default_config() for each config option; we will do so
> > inside of a skeleton config callback so that we know where to add
> > configuration handling later on when we need it.
> >
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> > diff --git a/builtin/walken.c b/builtin/walken.c
> > @@ -24,11 +25,36 @@ const char * const walken_usage[] = {
> >  static void init_walken_defaults(void)
> >  {
> >         /*
> > -        * We don't actually need the same components `git log` does; leave this
> > -        * empty for now.
> > +        * We don't use any other components or have settings to initialize, so
> > +        * leave this empty.
> >          */
> >  }
> 
> Meh, I don't think this change has anything to do with this patch. If
> the rewritten text is the one you prefer, then just introduce it like
> that in patch 3/13 where the function itself was introduced.

Whoops, yeah. I removed this change, I don't think it's significant.

Patch
diff mbox series

diff --git a/builtin/walken.c b/builtin/walken.c
index daae4f811a..2474a0d7b2 100644
--- a/builtin/walken.c
+++ b/builtin/walken.c
@@ -5,6 +5,7 @@ 
  */
 
 #include "builtin.h"
+#include "config.h"
 #include "parse-options.h"
 
 /*
@@ -24,11 +25,36 @@  const char * const walken_usage[] = {
 static void init_walken_defaults(void)
 {
 	/*
-	 * We don't actually need the same components `git log` does; leave this
-	 * empty for now.
+	 * We don't use any other components or have settings to initialize, so
+	 * leave this empty.
 	 */
 }
 
+/*
+ * This method will be called back by git_config(). It is used to gather values
+ * from the configuration files available to Git.
+ *
+ * Each time git_config() finds a configuration file entry, it calls this
+ * callback. Then, this function should compare it to entries which concern us,
+ * and make settings changes as necessary.
+ *
+ * If we are called with a config setting we care about, we should use one of
+ * the helpers which exist in config.h to pull out the value for ourselves, i.e.
+ * git_config_string(...) or git_config_bool(...).
+ *
+ * If we don't match anything, we should pass it along to another stakeholder
+ * who may otherwise care - in log's case, grep, gpg, and diff-ui. For our case,
+ * we'll ignore everybody else.
+ */
+static int git_walken_config(const char *var, const char *value, void *cb)
+{
+	/*
+	 * For now, we don't have any custom configuration, so fall back on the
+	 * default config.
+	 */
+	return git_default_config(var, value, cb);
+}
+
 int cmd_walken(int argc, const char **argv, const char *prefix)
 {
 	struct option options[] = {
@@ -43,6 +69,8 @@  int cmd_walken(int argc, const char **argv, const char *prefix)
 
 	init_walken_defaults();
 
+	git_config(git_walken_config, NULL);
+
 	/*
 	 * This line is "human-readable" and we are writing a plumbing command,
 	 * so we localize it and use the trace library to print only when