quick fix: a blank plugin option should be true, not false

contrib/pyln-client/pyln/client/plugin.py

@@ -250,8 +250,8 @@ def add_option(self, name, default, description, opt_type="string"):
                 "Name {} is already used by another option".format(name)
             )
 
-        if opt_type not in ["string", "int", "bool"]:
-            raise ValueError('{} not in supported type set (string, int, bool)')
+        if opt_type not in ["string", "int", "bool", "flag"]:
+            raise ValueError('{} not in supported type set (string, int, bool, flag)')
 
         self.options[name] = {
             'name': name,
@@ -261,6 +261,15 @@ def add_option(self, name, default, description, opt_type="string"):
             'value': None,
         }
 
+    def add_flag_option(self, name, description):
+        """Add a flag option that we'd like to register with lightningd.
+
+        Needs to be called before `Plugin.run`, otherwise we might not
+        end up getting it set.
+
+        """
+        self.add_option(name, None, description, opt_type="flag")
+
     def get_option(self, name):
         if name not in self.options:
             raise ValueError("No option with name {} registered".format(name))

doc/PLUGINS.md

@@ -104,7 +104,7 @@ this example:
 The `options` will be added to the list of command line options that
 `lightningd` accepts. The above will add a `--greeting` option with a
 default value of `World` and the specified description. *Notice that
-currently string, (unsigned) integers, and bool options are supported.*
+currently string, integers, bool, and flag options are supported.*
 
 The `rpcmethods` are methods that will be exposed via `lightningd`'s
 JSON-RPC over Unix-Socket interface, just like the builtin
@@ -139,6 +139,48 @@ methods, such as `help` and `getinfo`, as well as methods registered
 by other plugins. If there is a conflict then `lightningd` will report
 an error and exit.
 
+#### Types of Options
+
+There are currently four supported option 'types':
+  - string: a string
+  - bool: a boolean
+  - int: parsed as a signed integer (64-bit)
+  - flag: no-arg flag option. Is boolean under the hood. Defaults to false.
+
+Nota bene: if a `flag` type option is not set, it will not appear
+in the options set that is passed to the plugin.
+
+Here's an example option set, as sent in response to `getmanifest`
+
+```json
+  "options": [
+    {
+      "name": "greeting",
+      "type": "string",
+      "default": "World",
+      "description": "What name should I call you?"
+    },
+    {
+      "name": "run-hot",
+      "type": "flag",
+      "default": None,  // defaults to false
+      "description": "If set, overclocks plugin"
+    },
+    {
+      "name": "is_online",
+      "type": "bool",
+      "default": false,
+      "description": "Set to true if plugin can use network"
+    },
+    {
+      "name": "service-port",
+      "type": "int",
+      "default": 6666,
+      "description": "Port to use to connect to 3rd-party service"
+    }
+  ],
+```
+
 ### The `init` method
 
 The `init` method is required so that `lightningd` can pass back the

lightningd/options.c

@@ -1272,7 +1272,8 @@ static void add_config(struct lightningd *ld,
 			json_add_opt_log_levels(response, ld->log);
 		} else if (opt->cb_arg == (void *)opt_add_plugin_dir
 			   || opt->cb_arg == (void *)opt_disable_plugin
-			   || opt->cb_arg == (void *)plugin_opt_set) {
+			   || opt->cb_arg == (void *)plugin_opt_set
+			   || opt->cb_arg == (void *)plugin_opt_flag_set) {
 			/* FIXME: We actually treat it as if they specified
 			 * --plugin for each one, so ignore these */
 #if DEVELOPER

lightningd/plugin.c

@@ -464,6 +464,13 @@ struct io_plan *plugin_stdout_conn_init(struct io_conn *conn,
 			       plugin_read_json, plugin);
 }
 
+char *plugin_opt_flag_set(struct plugin_opt *popt)
+{
+	/* A set flag is a true */
+	*popt->value->as_bool = true;
+	return NULL;
+}
+
 char *plugin_opt_set(const char *arg, struct plugin_opt *popt)
 {
 	char *endp;
@@ -490,7 +497,7 @@ char *plugin_opt_set(const char *arg, struct plugin_opt *popt)
 		if (streq(arg, "true") || streq(arg, "True") || streq(arg, "1")) {
 			*popt->value->as_bool = true;
 		} else if (streq(arg, "false") || streq(arg, "False")
-				|| streq(arg, "0") || streq(arg, "")) {
+				|| streq(arg, "0")) {
 			*popt->value->as_bool = false;
 		} else
 			return tal_fmt(tmpctx, "%s does not parse as type %s",
@@ -551,15 +558,29 @@ static bool plugin_opt_add(struct plugin *plugin, const char *buffer,
 					popt, "%.*s (default: %s)", desctok->end - desctok->start,
 					buffer + desctok->start, *popt->value->as_bool ? "true" : "false");
 		}
+	} else if (json_tok_streq(buffer, typetok, "flag")) {
+		popt->type = "flag";
+		popt->value->as_bool = talz(popt->value, bool);
+		popt->description = json_strdup(popt, buffer, desctok);
+		/* We default flags to false, the default token is ignored */
+		*popt->value->as_bool = false;
+
 	} else {
-		plugin_kill(plugin, "Only \"string\", \"int\", and \"bool\" options are supported");
+		plugin_kill(plugin, "Only \"string\", \"int\", \"bool\", and \"flag\" options are supported");
 		return false;
 	}
 	if (!defaulttok)
 		popt->description = json_strdup(popt, buffer, desctok);
 	list_add_tail(&plugin->plugin_opts, &popt->list);
-	opt_register_arg(popt->name, plugin_opt_set, NULL, popt,
-			 popt->description);
+
+	if (streq(popt->type, "flag"))
+		opt_register_noarg(popt->name, plugin_opt_flag_set, popt,
+				   popt->description);
+
+	else
+		opt_register_arg(popt->name, plugin_opt_set, NULL, popt,
+				 popt->description);
+
 	return true;
 }
 
@@ -1127,6 +1148,11 @@ plugin_populate_init_request(struct plugin *plugin, struct jsonrpc_request *req)
 		/* Trim the `--` that we added before */
 		name = opt->name + 2;
 		if (opt->value->as_bool) {
+			/* We don't include 'flag' types if they're not
+			 * flagged on */
+			if (streq(opt->type, "flag") && !*opt->value->as_bool)
+				continue;
+
 			json_add_bool(req->stream, name, *opt->value->as_bool);
 			if (!deprecated_apis)
 				continue;

lightningd/plugin.h

@@ -254,6 +254,12 @@ void plugin_request_send(struct plugin *plugin,
  */
 char *plugin_opt_set(const char *arg, struct plugin_opt *popt);
 
+/**
+ * Callback called when plugin flag-type options.It just stores
+ * the value in the plugin_opt
+ */
+char *plugin_opt_flag_set(struct plugin_opt *popt);
+
 /**
  * Helpers to initialize a connection to a plugin; we read from their
  * stdout, and write to their stdin.

tests/plugins/options.py

@@ -17,4 +17,5 @@ def init(configuration, options, plugin):
 plugin.add_option('str_opt', 'i am a string', 'an example string option')
 plugin.add_option('int_opt', 7, 'an example int type option', opt_type='int')
 plugin.add_option('bool_opt', True, 'an example bool type option', opt_type='bool')
+plugin.add_flag_option('flag_opt', 'an example flag type option')
 plugin.run()

tests/test_plugin.py

@@ -58,19 +58,23 @@ def test_option_types(node_factory):
         'bool_opt': True,
     })
 
-    n.daemon.is_in_log(r"option str_opt ok <class 'str'>")
-    n.daemon.is_in_log(r"option int_opt 22 <class 'int'>")
-    n.daemon.is_in_log(r"option bool_opt True <class 'bool'>")
+    assert n.daemon.is_in_log(r"option str_opt ok <class 'str'>")
+    assert n.daemon.is_in_log(r"option int_opt 22 <class 'int'>")
+    assert n.daemon.is_in_log(r"option bool_opt True <class 'bool'>")
+    # flag options aren't passed through if not flagged on
+    assert not n.daemon.is_in_log(r"option flag_opt")
     n.stop()
 
     # A blank bool_opt should default to false
     n = node_factory.get_node(options={
         'plugin': plugin_path, 'str_opt': 'ok',
         'int_opt': 22,
-        'bool_opt': '',
+        'bool_opt': 'true',
+        'flag_opt': None,
     })
 
-    n.daemon.is_in_log(r"option bool_opt False <class 'bool'>")
+    assert n.daemon.is_in_log(r"option bool_opt True <class 'bool'>")
+    assert n.daemon.is_in_log(r"option flag_opt True <class 'bool'>")
     n.stop()
 
     # What happens if we give it a bad bool-option?
@@ -97,19 +101,36 @@ def test_option_types(node_factory):
     assert not n.daemon.running
     assert n.daemon.is_in_stderr('--int_opt: notok does not parse as type int')
 
+    # Flag opts shouldn't allow any input
+    n = node_factory.get_node(options={
+        'plugin': plugin_path,
+        'str_opt': 'ok',
+        'int_opt': 11,
+        'bool_opt': 1,
+        'flag_opt': True,
+    }, may_fail=True, expect_fail=True)
+
+    # the node should fail to start, and we get a stderr msg
+    assert not n.daemon.running
+    assert n.daemon.is_in_stderr("--flag_opt: doesn't allow an argument")
+
     plugin_path = os.path.join(os.getcwd(), 'tests/plugins/options.py')
     n = node_factory.get_node(options={
         'plugin': plugin_path,
         'str_opt': 'ok',
         'int_opt': 22,
         'bool_opt': 1,
+        'flag_opt': None,
+        'allow-deprecated-apis': True
     })
 
-    n.daemon.is_in_log(r"option str_opt ok <class 'str'>")
-    n.daemon.is_in_log(r"option int_opt 22 <class 'int'>")
-    n.daemon.is_in_log(r"option int_opt 22 <class 'str'>")
-    n.daemon.is_in_log(r"option bool_opt True <class 'bool'>")
-    n.daemon.is_in_log(r"option bool_opt true <class 'str'>")
+    # because of how the python json parser works, since we're adding the deprecated
+    # string option after the 'typed' option in the JSON, the string option overwrites
+    # the earlier typed option in JSON parsing, resulting in a option set of only strings
+    assert n.daemon.is_in_log(r"option str_opt ok <class 'str'>")
+    assert n.daemon.is_in_log(r"option int_opt 22 <class 'str'>")
+    assert n.daemon.is_in_log(r"option bool_opt 1 <class 'str'>")
+    assert n.daemon.is_in_log(r"option flag_opt True <class 'bool'>")
     n.stop()