From 21ac37170e55dcbf3fa060d7635bf7775bb07b2e Mon Sep 17 00:00:00 2001
From: muxator <a.mux@inwind.it>
Date: Thu, 21 Mar 2019 22:18:59 +0100
Subject: [PATCH] doc: rephrase settings.json.template and Settings.js

Better document current behaviour.
In this revision, ENV_VAR are supported, default values are not.
---
 settings.json.template     | 30 ++++++++++++++++--------------
 src/node/utils/Settings.js | 29 ++++++++++++++++++++---------
 2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/settings.json.template b/settings.json.template
index 50a095bac..74f5872dd 100644
--- a/settings.json.template
+++ b/settings.json.template
@@ -11,7 +11,7 @@
  * =================================
  *
  * All the configuration values can be read from environment variables using the
- * syntax "${ENV_VAR_NAME}".
+ * syntax "${ENV_VAR}".
  * This is useful, for example, when running in a Docker container.
  *
  * EXAMPLE:
@@ -23,21 +23,23 @@
  * variables PORT, MINIFY and SKIN_NAME.
  *
  * REMARKS:
- * Please note that a variable substitution always needs to be quoted.
- *    "port":   9001,          <-- Literal values. When not using substitution,
- *    "minify": false              only strings must be quoted: booleans and
- *    "skin":   "colibris"         numbers must not.
+ * Please note that variable substitution always needs to be quoted.
  *
- *    "port":   ${PORT}        <-- ERROR: this is not valid json
- *    "minify": ${MINIFY}
- *    "skin":   ${SKIN_NAME}
+ *    "port":     9001,          <-- Literal values. When not using
+ *    "minify":   false              substitution, only strings must be quoted.
+ *    "skinName": "colibris"         Booleans and numbers must not.
+ *
+ *    "port":     "${PORT}"      <-- CORRECT: if you want to use a variable
+ *    "minify":   "${MINIFY}"        substitution, put quotes around its name,
+ *    "skinName": "${SKIN_NAME}"     even if the required value is a number or a
+ *                                   boolean.
+ *                                   Etherpad will take care of rewriting it to
+ *                                   the proper type if necessary.
+ *
+ *    "port":     ${PORT}        <-- ERROR: this is not valid json. Quotes
+ *    "minify":   ${MINIFY}          around variable names are missing.
+ *    "skinName": ${SKIN_NAME}
  *
- *    "port":   "${PORT}"      <-- CORRECT: if you want to use a variable
- *    "minify": "${MINIFY}"        substitution, put quotes around its name,
- *    "skin":   "${SKIN_NAME}"     even if the required value is a number or a
- *                                 boolean.
- *                                 Etherpad will take care of rewriting it to
- *                                 the proper type if necessary.
  */
 {
   /*
diff --git a/src/node/utils/Settings.js b/src/node/utils/Settings.js
index d013d4222..c3b9d21e4 100644
--- a/src/node/utils/Settings.js
+++ b/src/node/utils/Settings.js
@@ -372,13 +372,13 @@ function storeSettings(settingsObj) {
 
 /**
  * Takes a javascript object containing Etherpad's configuration, and returns
- * another object, in which all the string properties whose name is of the form
- * "${ENV_VAR}", got their value replaced with the value of the given
+ * another object, in which all the string properties whose value is of the form
+ * "${ENV_VAR}" got their value replaced with the contents of the given
  * environment variable.
  *
- * An environment variable's value is always a string. However, the code base
- * makes use of the various json types. To maintain compatiblity, some
- * heuristics is applied:
+ * By definition, an environment variable's value is always a string. However,
+ * the code base makes use of the various json types. To maintain compatiblity,
+ * some heuristics is applied:
  *
  * - if ENV_VAR does not exist in the environment, null is returned;
  * - if ENV_VAR's value is "true" or "false", it is converted to the js boolean
@@ -386,10 +386,21 @@ function storeSettings(settingsObj) {
  * - if ENV_VAR's value looks like a number, it is converted to a js number
  *   (details in the code).
  *
- * Variable substitution is performed doing a round trip conversion to/from
- * json, using a custom replacer parameter in JSON.stringify(), and parsing the
- * JSON back again. This ensures that environment variable replacement is
- * performed even on nested objects.
+ * The following is a scheme of the behaviour of this function:
+ *
+ * +---------------------------+---------------+------------------+
+ * | Configuration string in   | Value of      | Resulting confi- |
+ * | settings.json             | ENV_VAR       | guration value   |
+ * |---------------------------|---------------|------------------|
+ * | "${ENV_VAR}"              | "some_string" | "some_string"    |
+ * | "${ENV_VAR}"              | "9001"        | 9001             |
+ * | "${ENV_VAR}"              | undefined     | null             |
+ * +---------------------------+---------------+------------------+
+ *
+ * IMPLEMENTATION NOTE: variable substitution is performed doing a round trip
+ *     conversion to/from json, using a custom replacer parameter in
+ *     JSON.stringify(), and parsing the JSON back again. This ensures that
+ *     environment variable replacement is performed even on nested objects.
  *
  * see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter
  */