[PATCH 3 of 3 PoC] cli: convert 'gearbox make-config' to 'kallithea config create'

[Mads Kiilerich]

On 9/24/18 10:45 PM, Thomas De Schampheleire wrote:
> # HG changeset patch
> # User Thomas De Schampheleire <thomas.de_schampheleire at nokia.com>
> # Date 1537821464 -7200
> #      Mon Sep 24 22:37:44 2018 +0200
> # Node ID 3c33619af70545eb9b3cbba585181094df67afbb
> # Parent  b807b1e90e61d2c8c16c5e3ec5978ef7dd7a2d31
> cli: convert 'gearbox make-config' to 'kallithea config create'


Intuitively, I think I would find 'kallithea-config --create' more 
obvious. But that might just be a habit. I guess it makes sense with 
separate "sub-command area" and "sub-command" ... and then "--arguments" 
to that. So ... just a thought ...

The main remaining doubt is as mentioned before, if the bare command 
name "kallithea" is misleading.


> Notes regarding the original implementation:
> - show_defaults was trying to obtain a custom
> value from mako_variable_values, which would never happen because
> the argument parser would not allow passing custom key-value pairs in
> combination with '--show-defaults'.
>
> - variables TMPL_FILE and here were no longer used


Right. I will land fixes for these issues. You can probably just take 
your side when rebasing.


> TODO: documentation update
>
> diff --git a/kallithea/bin/kallithea_cli.py b/kallithea/bin/kallithea_cli.py
> --- a/kallithea/bin/kallithea_cli.py
> +++ b/kallithea/bin/kallithea_cli.py
> @@ -14,6 +14,7 @@
>   
>   import click
>   
> +from kallithea.bin.kallithea_cli_config import config
>   from kallithea.bin.kallithea_cli_frontend import frontend
>   
>   @click.group()
> @@ -21,4 +22,5 @@ def cli():
>       """Various commands to set up a Kallithea instance."""
>       pass
>   
> +cli.add_command(config)
>   cli.add_command(frontend)
> diff --git a/kallithea/lib/paster_commands/make_config.py b/kallithea/bin/kallithea_cli_config.py
> rename from kallithea/lib/paster_commands/make_config.py
> rename to kallithea/bin/kallithea_cli_config.py
> --- a/kallithea/lib/paster_commands/make_config.py
> +++ b/kallithea/bin/kallithea_cli_config.py
> @@ -11,38 +11,40 @@
>   #
>   # You should have received a copy of the GNU General Public License
>   # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> -"""
> -kallithea.lib.paster_commands.make_config
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   
> -make-config gearbox command for Kallithea
> -
> -:license: GPLv3, see LICENSE.md for more details.
> -"""
> -
> -
> +import click
>   import os
> -import sys
>   import uuid
> -import argparse
>   from collections import defaultdict
>   
>   import mako.exceptions
>   
> -TMPL = 'template.ini.mako'
> -here = os.path.dirname(os.path.abspath(__file__))
> -
> -from kallithea.lib.paster_commands.common import BasePasterCommand
>   from kallithea.lib import inifile
>   
> + at click.group()
> +def config():
> +    pass
>   
> -class Command(BasePasterCommand):
> -    """Kallithea: Create a new config file
> +def show_defaults(ctx, param, value):
> +    if not value or ctx.resilient_parsing:


That one is not entirely obvious ... and especially why a bare return si 
the right response. Should it have a comment with an explanation ... or 
a hint about what to look for in the documentation?


> +        return
> +
> +    for key, value in inifile.default_variables.items():
> +        click.echo('%s=%s' % (key, value))
>   
> -    make-config is part of a two-phase installation process (the
> -    second phase is setup-app). make-config creates a bare configuration
> -    file (possibly filling in defaults from the extra
> -    variables you give).
> +    ctx.exit()
> +
> + at config.command()
> + at click.option('--show-defaults', callback=show_defaults,
> +              is_flag=True, expose_value=False, is_eager=True,
> +              help='Show the default values that can be overridden')
> + at click.argument('config_file', type=click.Path())
> + at click.argument('key_value_pairs', nargs=-1)
> +def create(config_file, key_value_pairs):
> +    """Create a new configuration file
> +
> +    This command creates a bare configuration file (possibly filling in
> +    defaults from the extra variables you give).


What does "bare" mean here? It is actually quite verbose in my opinion.


/Mads