From 953410494248de45118dbadae9e603970ad4e74a Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 6 Mar 2023 05:32:55 +0100 Subject: [PATCH 1/6] Support hierarchical loading of the config file --- spec/ameba/cli/cmd_spec.cr | 10 ++----- spec/ameba/formatter/todo_formatter_spec.cr | 2 +- src/ameba/cli/cmd.cr | 5 ++-- src/ameba/config.cr | 30 ++++++++++++++++++--- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/spec/ameba/cli/cmd_spec.cr b/spec/ameba/cli/cmd_spec.cr index 6f058f7c..90f9f289 100644 --- a/spec/ameba/cli/cmd_spec.cr +++ b/spec/ameba/cli/cmd_spec.cr @@ -21,7 +21,7 @@ module Ameba::Cli %w(-c --config).each do |f| it "accepts #{f} flag" do c = Cli.parse_args [f, "config.yml"] - c.config.should eq "config.yml" + c.config.should eq Path["config.yml"] end end @@ -82,12 +82,6 @@ module Ameba::Cli c.colors?.should be_true end - it "ignores --config if --gen-config flag passed" do - c = Cli.parse_args %w(--gen-config --config my_config.yml) - c.formatter.should eq :todo - c.config.should eq "" - end - describe "-e/--explain" do it "configures file/line/column" do c = Cli.parse_args %w(--explain src/file.cr:3:5) @@ -166,7 +160,7 @@ module Ameba::Cli c.globs.should be_nil c.only.should be_nil c.except.should be_nil - c.config.should eq Config::PATH + c.config.should be_nil end end end diff --git a/spec/ameba/formatter/todo_formatter_spec.cr b/spec/ameba/formatter/todo_formatter_spec.cr index ec9fdf99..3b13f78b 100644 --- a/spec/ameba/formatter/todo_formatter_spec.cr +++ b/spec/ameba/formatter/todo_formatter_spec.cr @@ -45,7 +45,7 @@ module Ameba s = Source.new "a = 1", "source.cr" s.add_issue DummyRule.new, {1, 2}, "message" formatter.finished([s]) - io.to_s.should contain "Created .ameba.yml" + io.to_s.should contain "Created #{Config::PATH}" end end diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr index 6cebd608..67d19b16 100644 --- a/src/ameba/cli/cmd.cr +++ b/src/ameba/cli/cmd.cr @@ -44,7 +44,7 @@ module Ameba::Cli end private class Opts - property config = Config::PATH + property config : Path? property formatter : Symbol | String | Nil property globs : Array(String)? property only : Array(String)? @@ -76,7 +76,7 @@ module Ameba::Cli parser.on("-c", "--config PATH", "Specify a configuration file") do |path| - opts.config = path unless opts.config.empty? + opts.config = Path[path] unless path.empty? end parser.on("-f", "--format FORMATTER", @@ -105,7 +105,6 @@ module Ameba::Cli parser.on("--gen-config", "Generate a configuration file acting as a TODO list") do opts.formatter = :todo - opts.config = "" end parser.on("--fail-level SEVERITY", diff --git a/src/ameba/config.cr b/src/ameba/config.cr index fd5eca2e..a0fc3ad8 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -24,7 +24,12 @@ class Ameba::Config json: Formatter::JSONFormatter, } - PATH = ".ameba.yml" + DEFAULT_PATHS = { + "~/.ameba.yml", + "~/.config/ameba/config.yml", + } + FILENAME = ".ameba.yml" + PATH = Path[Dir.current] / FILENAME DEFAULT_GLOBS = %w( **/*.cr @@ -77,14 +82,33 @@ class Ameba::Config # ``` # config = Ameba::Config.load # ``` - def self.load(path = PATH, colors = true) + def self.load(path = nil, colors = true) Colorize.enabled = colors - content = File.exists?(path) ? File.read path : "{}" + content = read_config(path) || "{}" Config.new YAML.parse(content) rescue e raise "Config file is invalid: #{e.message}" end + protected def self.read_config(path) : String? + if path + return File.exists?(path) ? File.read(path) : nil + end + path = Path[PATH].expand(home: true) + + search_paths = path + .parents + .map! { |search_path| search_path / FILENAME } + + search_paths.reverse_each do |search_path| + return File.read(search_path) if File.exists?(search_path) + end + + DEFAULT_PATHS.each do |default_path| + return File.read(default_path) if File.exists?(default_path) + end + end + def self.formatter_names AVAILABLE_FORMATTERS.keys.join('|') end From 749da0984c4a1786bd4c81199bd26e15b5e76329 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 6 Mar 2023 05:47:30 +0100 Subject: [PATCH 2/6] Rename `Config::PATH` to `Config::DEFAULT_PATH` --- spec/ameba/formatter/todo_formatter_spec.cr | 4 ++-- src/ameba/config.cr | 6 +++--- src/ameba/formatter/todo_formatter.cr | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/ameba/formatter/todo_formatter_spec.cr b/spec/ameba/formatter/todo_formatter_spec.cr index 3b13f78b..99ad3949 100644 --- a/spec/ameba/formatter/todo_formatter_spec.cr +++ b/spec/ameba/formatter/todo_formatter_spec.cr @@ -20,7 +20,7 @@ module Ameba describe Formatter::TODOFormatter do ::Spec.after_each do - FileUtils.rm_rf(Ameba::Config::PATH) + FileUtils.rm_rf(Ameba::Config::DEFAULT_PATH) end context "problems not found" do @@ -45,7 +45,7 @@ module Ameba s = Source.new "a = 1", "source.cr" s.add_issue DummyRule.new, {1, 2}, "message" formatter.finished([s]) - io.to_s.should contain "Created #{Config::PATH}" + io.to_s.should contain "Created #{Config::DEFAULT_PATH}" end end diff --git a/src/ameba/config.cr b/src/ameba/config.cr index a0fc3ad8..481d3bda 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -28,8 +28,8 @@ class Ameba::Config "~/.ameba.yml", "~/.config/ameba/config.yml", } - FILENAME = ".ameba.yml" - PATH = Path[Dir.current] / FILENAME + FILENAME = ".ameba.yml" + DEFAULT_PATH = Path[Dir.current] / FILENAME DEFAULT_GLOBS = %w( **/*.cr @@ -94,7 +94,7 @@ class Ameba::Config if path return File.exists?(path) ? File.read(path) : nil end - path = Path[PATH].expand(home: true) + path = Path[DEFAULT_PATH].expand(home: true) search_paths = path .parents diff --git a/src/ameba/formatter/todo_formatter.cr b/src/ameba/formatter/todo_formatter.cr index eaee9efc..8bcc3d5c 100644 --- a/src/ameba/formatter/todo_formatter.cr +++ b/src/ameba/formatter/todo_formatter.cr @@ -26,7 +26,7 @@ module Ameba::Formatter end private def generate_todo_config(issues) - file = File.new(Config::PATH, mode: "w") + file = File.new(Config::DEFAULT_PATH, mode: "w") file << header rule_issues_map(issues).each do |rule, rule_issues| file << "\n# Problems found: #{rule_issues.size}" From 102e2834b64d4940237fe77586e68ae940438796 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 6 Mar 2023 20:18:49 +0100 Subject: [PATCH 3/6] Honor `XDG_CONFIG_HOME` ENV variable Following XDG Spec: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html --- src/ameba/config.cr | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 481d3bda..55d57d86 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -24,9 +24,10 @@ class Ameba::Config json: Formatter::JSONFormatter, } - DEFAULT_PATHS = { - "~/.ameba.yml", - "~/.config/ameba/config.yml", + XDG_CONFIG_HOME = ENV.fetch("XDG_CONFIG_HOME", "~/.config") + DEFAULT_PATHS = { + Path["~/.ameba.yml"], + Path[XDG_CONFIG_HOME] / "ameba/config.yml", } FILENAME = ".ameba.yml" DEFAULT_PATH = Path[Dir.current] / FILENAME From e481a8d139e7b8358e3f8930dc65dfe9f8d47aa6 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 6 Mar 2023 20:42:58 +0100 Subject: [PATCH 4/6] Use `FILENAME` constant consistently --- src/ameba/config.cr | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 55d57d86..7c2ba585 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -25,12 +25,13 @@ class Ameba::Config } XDG_CONFIG_HOME = ENV.fetch("XDG_CONFIG_HOME", "~/.config") - DEFAULT_PATHS = { - Path["~/.ameba.yml"], + + FILENAME = ".ameba.yml" + DEFAULT_PATH = Path[Dir.current] / FILENAME + DEFAULT_PATHS = { + Path["~"] / FILENAME, Path[XDG_CONFIG_HOME] / "ameba/config.yml", } - FILENAME = ".ameba.yml" - DEFAULT_PATH = Path[Dir.current] / FILENAME DEFAULT_GLOBS = %w( **/*.cr From 8d56f22af1b6c518c94317d2c2ca65df1fc57c44 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 6 Mar 2023 21:19:47 +0100 Subject: [PATCH 5/6] Document the new `Config.load` behaviour --- src/ameba/config.cr | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 7c2ba585..c54eefcf 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -10,8 +10,30 @@ require "./glob_utils" # config.formatter = my_formatter # ``` # -# By default config loads `.ameba.yml` file in a current directory. +# By default config loads `.ameba.yml` file located in a current +# working directory. # +# If it cannot be found until reaching the root directory, then it will be +# searched for in the user’s global config locations, which consists of a +# dotfile or a config file inside the XDG Base Directory specification. +# +# - `~/.ameba.yml` +# - `$XDG_CONFIG_HOME/ameba/config.yml` (expands to `~/.config/ameba/config.yml` +# if `$XDG_CONFIG_HOME` is not set) +# +# If both files exist, the dotfile will be selected. +# +# As an example, if Ameba is invoked from inside `/path/to/project/lib/utils`, +# then it will use the config as specified inside the first of the following files: +# +# - `/path/to/project/lib/utils/.ameba.yml` +# - `/path/to/project/lib/.ameba.yml` +# - `/path/to/project/.ameba.yml` +# - `/path/to/.ameba.yml` +# - `/path/.ameba.yml` +# - `/.ameba.yml` +# - `~/.ameba.yml` +# - `~/.config/ameba/config.yml` class Ameba::Config include GlobUtils From 4c59858f25b2b10be110f03c9690eabf0683d64f Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 6 Mar 2023 21:22:47 +0100 Subject: [PATCH 6/6] Apply code review suggestions --- src/ameba/config.cr | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/ameba/config.cr b/src/ameba/config.cr index c54eefcf..a8e9e537 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -114,22 +114,25 @@ class Ameba::Config raise "Config file is invalid: #{e.message}" end - protected def self.read_config(path) : String? + protected def self.read_config(path = nil) if path return File.exists?(path) ? File.read(path) : nil end + each_config_path do |config_path| + return File.read(config_path) if File.exists?(config_path) + end + end + + protected def self.each_config_path(&) path = Path[DEFAULT_PATH].expand(home: true) - search_paths = path - .parents - .map! { |search_path| search_path / FILENAME } - + search_paths = path.parents search_paths.reverse_each do |search_path| - return File.read(search_path) if File.exists?(search_path) + yield search_path / FILENAME end DEFAULT_PATHS.each do |default_path| - return File.read(default_path) if File.exists?(default_path) + yield default_path end end