From 2e94ec60b5bab63b6ea625ae336e56e8b503b353 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Thu, 10 Oct 2024 14:36:30 -0400 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Simplified=20logging=20fil?= =?UTF-8?q?e=20CLI=20options?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- bin/cli.rb | 45 +++++++++++++++++++++++++++++++-------------- bin/cli_handler.rb | 6 ++++-- bin/cli_helper.rb | 23 +++++++++++++---------- 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/bin/cli.rb b/bin/cli.rb index b48b3443..1b56a4a0 100644 --- a/bin/cli.rb +++ b/bin/cli.rb @@ -138,6 +138,9 @@ module CeedlingTasks specified YAML file. See documentation for complete details. \x5> --mixin my_compiler --mixin my/path/mixin.yml" + # Intentionally disallowed Linux/Unix/Windows filename characters to avoid mistakenly filtering --logfile default + MISSING_LOGFILE_DEFAULT = "/<>\\||*" + class CLI < Thor include Thor::Actions extend PermissiveCLI @@ -278,9 +281,10 @@ def upgrade(path) method_option :project, :type => :string, :default => nil, :aliases => ['-p'], :desc => DOC_PROJECT_FLAG method_option :mixin, :type => :string, :default => [], :repeatable => true, :aliases => ['-m'], :desc => DOC_MIXIN_FLAG method_option :verbosity, :type => :string, :default => VERBOSITY_NORMAL, :aliases => ['-v'], :desc => "Sets logging level" - # --log defaults to nil so we can check if it's been set by user as part of --logfile mutually exclusive option check - method_option :log, :type => :boolean, :default => nil, :aliases => ['-l'], :desc => "Enable logging to default /logs/#{DEFAULT_CEEDLING_LOGFILE}" - method_option :logfile, :type => :string, :default => nil, :desc => "Enable logging to filepath (ex. foo/bar/mybuild.log)" + # :default, :lazy_default & :check_default_type: allow us to encode 3 possible logfile scenarios (see special handling comments below) + method_option :logfile, :type => :string, :aliases => ['-l'], + :default => false, :lazy_default => MISSING_LOGFILE_DEFAULT, :check_default_type => false, + :desc => "Enables logging to /logs/#{DEFAULT_CEEDLING_LOGFILE} if blank or to specified filepath" method_option :graceful_fail, :type => :boolean, :default => nil, :desc => "Force exit code of 0 for unit test failures" method_option :test_case, :type => :string, :default => '', :desc => "Filter for individual unit test names" method_option :exclude_test_case, :type => :string, :default => '', :desc => "Prevent matched unit test names from running" @@ -304,29 +308,41 @@ def upgrade(path) • `--test-case` and its inverse `--exclude-test-case` set test case name matchers to run only a subset of the unit test suite. See docs for full details. - - • The simple `--log` flag and more configurable `--logfile` are mutually - exclusive. It is valid to use one or the other but not both. LONGDESC ) ) def build(*tasks) # Get unfrozen copies so we can add / modify _options = options.dup() _options[:project] = options[:project].dup() if !options[:project].nil? - _options[:logfile] = options[:logfile].dup() _options[:mixin] = [] options[:mixin].each {|mixin| _options[:mixin] << mixin.dup() } _options[:verbosity] = VERBOSITY_DEBUG if options[:debug] - # Mutually exclusive options check (Thor does not offer option combination limitations) - # If :log is not nil, then the user set it. If :logfile is not empty, the user set it too. - if !options[:log].nil? and !options[:logfile].empty? - raise Thor::Error, "Use only one of --log(-l) or --logfile" + # Set something to be reset below + _options[:logfile] = nil + + # Handle the 3 logging option values: + # 1. No logging (default) + # 2. Enabling logging with default log filepath + # 3. Explicitly set log filepath + case options[:logfile] + + # Match against Thor's :lazy_default for --logging flag with missing value + when MISSING_LOGFILE_DEFAULT + # Enable logging to default log filepath + _options[:logfile] = true + + # Match against missing --logging flag + when false + # No logging + _options[:logfile] = false + + # Copy in explicitly provided filepath from --logging= + else + # Filepath to explicitly set log file + _options[:logfile] = options[:logfile].dup() end - # Force default of false since we use nil as default value in Thor option definition - _options[:log] = false if options[:log].nil? - @handler.build( env:ENV, app_cfg:@app_cfg, options:_options, tasks:tasks ) end @@ -396,6 +412,7 @@ def environment() @handler.environment( ENV, @app_cfg, _options ) end + desc "examples", "List available example projects" method_option :debug, :type => :boolean, :default => false, :hide => true long_desc( CEEDLING_HANDOFF_OBJECTS[:loginator].sanitize( diff --git a/bin/cli_handler.rb b/bin/cli_handler.rb index 756ba974..8222ec01 100644 --- a/bin/cli_handler.rb +++ b/bin/cli_handler.rb @@ -160,7 +160,9 @@ def upgrade_project(env, app_cfg, options, path) def build(env:, app_cfg:, options:{}, tasks:) @helper.set_verbosity( options[:verbosity] ) - @path_validator.standardize_paths( options[:project], options[:logfile], *options[:mixin] ) + @path_validator.standardize_paths( options[:project], *options[:mixin] ) + + @path_validator.standardize_paths( options[:logfile] ) if options[:logfile].class == String _, config = @configinator.loadinate( builtin_mixins:BUILTIN_MIXINS, filepath:options[:project], mixins:options[:mixin], env:env ) @@ -175,7 +177,7 @@ def build(env:, app_cfg:, options:{}, tasks:) ) logging_path = @helper.process_logging_path( config ) - log_filepath = @helper.process_log_filepath( options[:log], logging_path, options[:logfile] ) + log_filepath = @helper.process_log_filepath( logging_path, options[:logfile] ) @loginator.log( " > Logfile: #{log_filepath}" ) if !log_filepath.empty? diff --git a/bin/cli_helper.rb b/bin/cli_helper.rb index f6e69353..95e00331 100644 --- a/bin/cli_helper.rb +++ b/bin/cli_helper.rb @@ -184,18 +184,21 @@ def process_logging_path(config) end - def process_log_filepath(enabled, logging_path, filepath) - # No log file if neither enabled nor a specific filename/filepath - return '' if !enabled && (filepath.nil? || filepath.empty?) - - # Default logfile name (to be placed in default location of logging_path) if enabled but no filename/filepath - if (enabled && filepath.empty?) + def process_log_filepath(logging_path, filepath) + case filepath + # No logging + when false + return '' + + # Default logfile path if no filename/filepath + when true filepath = File.join( logging_path, DEFAULT_CEEDLING_LOGFILE ) - end - - # Otherwise, a filename/filepath was provided that implicitly enables logging + filepath = File.expand_path( filepath ) - filepath = File.expand_path( filepath ) + # Otherwise, explcit filename/filepath provided that implicitly enables logging + else + filepath = File.expand_path( filepath ) + end dir = File.dirname( filepath )