El mié., 26 sept. 2018 a las 13:01, Mads Kiilerich (<[email protected]>) escribió: > > On 9/24/18 10:45 PM, Thomas De Schampheleire wrote: > > # HG changeset patch > > # User Thomas De Schampheleire <[email protected]> > > # 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 ...
Comparing kallithea config create kallithea config --create I feel the first should be chosen. In particular if we have multiple commands within a topic, say (hypothetically) kallithea config create kallithea config update vs kallithea config --create kallithea config --update Suppose that these commands create/update also need some options, albeit different ones. Then you'd get: kallithea config create --foo --bar kallithea config update --baz vs kallithea config --create --foo --bar kallithea config --update --baz and that second set seems odd to me: foo/bar relate to the 'create' part, not to the 'config' thing. We could organize things differently, e.g. instead of one global 'kallithea' command with topics, you could create multiple global commands, e.g. kallithea-config kallithea-db kallithea-frontend kallithea-repo ... or in an other directory keep one global command but flatten the rest, e.g. kallithea config-create kallithea config-update kallithea db-create kallithea db-upgrade kallithea front-end-create etc. > > 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 > > > > [email protected]() > > +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? > Yes. The corresponding docs are; http://click.pocoo.org/6/options/#callbacks-and-eager-options "The resilient_parsing flag is applied to the context if Click wants to parse the command line without any destructive behavior that would change the execution flow. In this case, because we would exit the program, we instead do nothing." > > > + 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() > > + > > [email protected]() > > [email protected]('--show-defaults', callback=show_defaults, > > + is_flag=True, expose_value=False, is_eager=True, > > + help='Show the default values that can be overridden') > > [email protected]('config_file', type=click.Path()) > > [email protected]('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. > I guess you are aware that this comes from the original code. I think the intention of 'bare' is to say: without any real user modification yet (except for extra variables passed). I think the word can be removed without problem. /Thomas _______________________________________________ kallithea-general mailing list [email protected] https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
