From bb313904e9a588a0fe3394f62fa66a69938356ee Mon Sep 17 00:00:00 2001 From: Chris Fung Date: Tue, 21 Feb 2017 21:09:54 -0800 Subject: [PATCH 1/2] Adds configurable option to set message format This allows the format of the messages produced by each formatter to be configured by specifying a format string in the .pronto.yml config file. Format can be overridden on a global basis by specifying the `format` key at the top-level of the file, or it can be overridden on a per-formatter basis by nesting `format` keys under the formatter's name (as defined in `Formatter::FORMATTERS` constant). If a formatter does not have a specific format defined in the config file, it will fall back to the global format, and then fall back to the default format (which is just `"%{msg}"`). Format uses a Ruby format string and makes all of the attributes of the `Message` object available for interpolation. The `TextFormatter` additionally adds a special `TextMessageDecorator` to add colorized and specially formatted values for file location and level. These were in the original formatter code so I wanted to keep them. --- README.md | 30 +++++++++ lib/pronto.rb | 1 + lib/pronto/config.rb | 9 +++ lib/pronto/config_file.rb | 8 ++- lib/pronto/formatter/base.rb | 13 ++++ lib/pronto/formatter/checkstyle_formatter.rb | 2 +- lib/pronto/formatter/git_formatter.rb | 10 ++- lib/pronto/formatter/json_formatter.rb | 2 +- lib/pronto/formatter/null_formatter.rb | 2 +- lib/pronto/formatter/text_formatter.rb | 69 +++++++++++--------- lib/pronto/message.rb | 11 ++++ lib/pronto/runner.rb | 1 + spec/pronto/config_spec.rb | 17 +++++ 13 files changed, 135 insertions(+), 40 deletions(-) create mode 100644 lib/pronto/formatter/base.rb diff --git a/README.md b/README.md index a7d5c7a1..e7ccb99d 100644 --- a/README.md +++ b/README.md @@ -207,6 +207,36 @@ via environment variables. Their names will be the upcased path to the property. For example: `PRONTO_GITHUB_SLUG` or `PRONTO_GITLAB_API_PRIVATE_TOKEN`. Environment variables will always take precedence over values in configuration file. +### Message format + +Pronto allows you to configure the format of the messages that are produced. You +can set a default format that will be used by all formatters, or you can +configure a separate format per formatter, if you are using several. + +To change the default format: + +```yaml +format: "%{runner} %{level} %{msg}" +``` + +To add the title of the Runner to the GitHub Pull Request formatter only: + +```yaml +github_pr: + format: "%{runner} - %{msg}" +``` + +The available values to be interpolated into the message are: + +| Key | Description | +|--------------|----------------| +| `path` | File path. | +| `line` | Line number. | +| `level` | Message level. | +| `msg` | Message. | +| `commit_sha` | SHA. | +| `runner` | Runner name. | + ## Runners Pronto can run various tools and libraries, as long as there's a runner for it. diff --git a/lib/pronto.rb b/lib/pronto.rb index a1d87002..cf5a009b 100644 --- a/lib/pronto.rb +++ b/lib/pronto.rb @@ -29,6 +29,7 @@ require 'pronto/bitbucket' require 'pronto/formatter/colorizable' +require 'pronto/formatter/base' require 'pronto/formatter/text_formatter' require 'pronto/formatter/json_formatter' require 'pronto/formatter/git_formatter' diff --git a/lib/pronto/config.rb b/lib/pronto/config.rb index 02436a63..a52485fc 100644 --- a/lib/pronto/config.rb +++ b/lib/pronto/config.rb @@ -43,6 +43,15 @@ def max_warnings ENV['PRONTO_MAX_WARNINGS'] || @config_hash['max_warnings'] end + def message_format(formatter) + formatter_config = @config_hash[formatter] + if formatter_config && formatter_config.key?('format') + formatter_config['format'] + else + ENV["PRONTO_FORMAT"] || @config_hash['format'] + end + end + def logger @logger ||= begin verbose = ENV['PRONTO_VERBOSE'] || @config_hash['verbose'] diff --git a/lib/pronto/config_file.rb b/lib/pronto/config_file.rb index edb5eaba..8c0cfe69 100644 --- a/lib/pronto/config_file.rb +++ b/lib/pronto/config_file.rb @@ -1,5 +1,7 @@ module Pronto class ConfigFile + DEFAULT_MESSAGE_FORMAT = "%{msg}".freeze + EMPTY = { 'all' => { 'exclude' => [], @@ -22,10 +24,14 @@ class ConfigFile 'password' => nil, 'web_endpoint' => 'https://bitbucket.org/' }, + 'text' => { + 'format' => "%{color_location} %{color_level}: %{msg}" + }, 'runners' => [], 'formatters' => [], 'max_warnings' => nil, - 'verbose' => false + 'verbose' => false, + 'format' => DEFAULT_MESSAGE_FORMAT }.freeze def initialize(path = '.pronto.yml') diff --git a/lib/pronto/formatter/base.rb b/lib/pronto/formatter/base.rb new file mode 100644 index 00000000..7e72a173 --- /dev/null +++ b/lib/pronto/formatter/base.rb @@ -0,0 +1,13 @@ +module Pronto + module Formatter + class Base + def self.name + Formatter::FORMATTERS.invert[self] + end + + def config + @config ||= Config.new + end + end + end +end diff --git a/lib/pronto/formatter/checkstyle_formatter.rb b/lib/pronto/formatter/checkstyle_formatter.rb index 0b798c98..63cd4d36 100644 --- a/lib/pronto/formatter/checkstyle_formatter.rb +++ b/lib/pronto/formatter/checkstyle_formatter.rb @@ -2,7 +2,7 @@ module Pronto module Formatter - class CheckstyleFormatter + class CheckstyleFormatter < Base def initialize @output = '' end diff --git a/lib/pronto/formatter/git_formatter.rb b/lib/pronto/formatter/git_formatter.rb index 20caf397..0989adfe 100644 --- a/lib/pronto/formatter/git_formatter.rb +++ b/lib/pronto/formatter/git_formatter.rb @@ -1,6 +1,6 @@ module Pronto module Formatter - class GitFormatter + class GitFormatter < Base def format(messages, repo, patches) client = client_module.new(repo) existing = existing_comments(messages, client, repo) @@ -39,10 +39,6 @@ def grouped_comments(comments) comments.group_by { |comment| [comment.path, comment.position] } end - def config - @config ||= Config.new - end - def consolidate_comments(comments) comment = comments.first if comments.length > 1 @@ -65,7 +61,9 @@ def join_comments(comments) def new_comment(message, patches) config.logger.log("Creating a comment from message: #{message.inspect}") sha = message.commit_sha - body = message.msg + + body = config.message_format(self.class.name) % message.to_h + path = message.path lineno = line_number(message, patches) if message.line Comment.new(sha, body, path, lineno) diff --git a/lib/pronto/formatter/json_formatter.rb b/lib/pronto/formatter/json_formatter.rb index 8aee0989..1765a34d 100644 --- a/lib/pronto/formatter/json_formatter.rb +++ b/lib/pronto/formatter/json_formatter.rb @@ -2,7 +2,7 @@ module Pronto module Formatter - class JsonFormatter + class JsonFormatter < Base def format(messages, _, _) messages.map do |message| lineno = message.line.new_lineno if message.line diff --git a/lib/pronto/formatter/null_formatter.rb b/lib/pronto/formatter/null_formatter.rb index 41413738..2aa58447 100644 --- a/lib/pronto/formatter/null_formatter.rb +++ b/lib/pronto/formatter/null_formatter.rb @@ -1,6 +1,6 @@ module Pronto module Formatter - class NullFormatter + class NullFormatter < Base def format(_, _, _) end end diff --git a/lib/pronto/formatter/text_formatter.rb b/lib/pronto/formatter/text_formatter.rb index 1f274b45..97bc0bc0 100644 --- a/lib/pronto/formatter/text_formatter.rb +++ b/lib/pronto/formatter/text_formatter.rb @@ -1,44 +1,53 @@ module Pronto module Formatter - class TextFormatter - include Colorizable + class TextFormatter < Base + class TextMessageDecorator < SimpleDelegator + include Colorizable + + LOCATION_COLOR = :cyan + + LEVEL_COLORS = { + info: :yellow, + warning: :magenta, + error: :red, + fatal: :red + }.freeze + + def to_h + original = __getobj__.to_h + original[:color_level] = format_level(__getobj__) + original[:color_location] = format_location(__getobj__) + original + end - LOCATION_COLOR = :cyan + private - LEVEL_COLORS = { - info: :yellow, - warning: :magenta, - error: :red, - fatal: :red - }.freeze + def format_location(message) + line = message.line + lineno = line.new_lineno if line + path = message.path + commit_sha = message.commit_sha - def format(messages, _, _) - messages.map do |message| - "#{format_location(message)} #{format_level(message)}: #{message.msg}".strip + if path || lineno + path = colorize(path, LOCATION_COLOR) if path + "#{path}:#{lineno}" + elsif commit_sha + colorize(commit_sha[0..6], LOCATION_COLOR) + end end - end - - private - def format_location(message) - line = message.line - lineno = line.new_lineno if line - path = message.path - commit_sha = message.commit_sha + def format_level(message) + level = message.level + color = LEVEL_COLORS.fetch(level) - if path || lineno - path = colorize(path, LOCATION_COLOR) if path - "#{path}:#{lineno}" - elsif commit_sha - colorize(commit_sha[0..6], LOCATION_COLOR) + colorize(level[0].upcase, color) end end - def format_level(message) - level = message.level - color = LEVEL_COLORS.fetch(level) - - colorize(level[0].upcase, color) + def format(messages, _, _) + messages.map do |message| + (config.message_format(self.class.name) % TextMessageDecorator.new(message).to_h).strip + end end end end diff --git a/lib/pronto/message.rb b/lib/pronto/message.rb index cec1d76c..31b951eb 100644 --- a/lib/pronto/message.rb +++ b/lib/pronto/message.rb @@ -40,6 +40,17 @@ def hash end end + def to_h + { + path: path, + line: line, + level: level, + msg: msg, + commit_sha: commit_sha, + runner: @runner && @runner.title + } + end + private def comparison_attributes diff --git a/lib/pronto/runner.rb b/lib/pronto/runner.rb index dbfdaec8..8a55e632 100644 --- a/lib/pronto/runner.rb +++ b/lib/pronto/runner.rb @@ -5,6 +5,7 @@ class Runner def initialize(patches, commit = nil) @patches = patches @commit = commit + @config = Config.new end def self.runners diff --git a/spec/pronto/config_spec.rb b/spec/pronto/config_spec.rb index 28265231..1e7c1ed3 100644 --- a/spec/pronto/config_spec.rb +++ b/spec/pronto/config_spec.rb @@ -55,5 +55,22 @@ module Pronto it { should == 'ruby/ruby' } end end + + describe '#message_format' do + subject { config.message_format('whatever') } + + context 'when there is an entry in the config file' do + let(:config_hash) { { 'whatever' => { 'format' => whatever_format } } } + let(:whatever_format) { "that's just like your opinion man" } + + it { should == whatever_format } + end + + context 'when there is no entry in the config file' do + let(:config_hash) { ConfigFile::EMPTY } + + it { should == ConfigFile::DEFAULT_MESSAGE_FORMAT } + end + end end end From e54f1d70098d5b19676b30e0a804747e7c07aa9a Mon Sep 17 00:00:00 2001 From: Chris Fung Date: Sat, 4 Mar 2017 20:00:05 -0800 Subject: [PATCH 2/2] Reorganize code according to comments --- README.md | 7 +++ lib/pronto/formatter/text_formatter.rb | 49 ++----------------- .../formatter/text_message_decorator.rb | 46 +++++++++++++++++ 3 files changed, 58 insertions(+), 44 deletions(-) create mode 100644 lib/pronto/formatter/text_message_decorator.rb diff --git a/README.md b/README.md index e7ccb99d..2f17c8b3 100644 --- a/README.md +++ b/README.md @@ -237,6 +237,13 @@ The available values to be interpolated into the message are: | `commit_sha` | SHA. | | `runner` | Runner name. | +The following values are available only to the text formatter: + +| Key | Description | +|------------------|--------------------------| +| `color_level` | Colorized message level. | +| `color_location` | Colorized location. | + ## Runners Pronto can run various tools and libraries, as long as there's a runner for it. diff --git a/lib/pronto/formatter/text_formatter.rb b/lib/pronto/formatter/text_formatter.rb index 97bc0bc0..f491ea15 100644 --- a/lib/pronto/formatter/text_formatter.rb +++ b/lib/pronto/formatter/text_formatter.rb @@ -1,52 +1,13 @@ +require 'pronto/formatter/text_message_decorator' + module Pronto module Formatter class TextFormatter < Base - class TextMessageDecorator < SimpleDelegator - include Colorizable - - LOCATION_COLOR = :cyan - - LEVEL_COLORS = { - info: :yellow, - warning: :magenta, - error: :red, - fatal: :red - }.freeze - - def to_h - original = __getobj__.to_h - original[:color_level] = format_level(__getobj__) - original[:color_location] = format_location(__getobj__) - original - end - - private - - def format_location(message) - line = message.line - lineno = line.new_lineno if line - path = message.path - commit_sha = message.commit_sha - - if path || lineno - path = colorize(path, LOCATION_COLOR) if path - "#{path}:#{lineno}" - elsif commit_sha - colorize(commit_sha[0..6], LOCATION_COLOR) - end - end - - def format_level(message) - level = message.level - color = LEVEL_COLORS.fetch(level) - - colorize(level[0].upcase, color) - end - end - def format(messages, _, _) messages.map do |message| - (config.message_format(self.class.name) % TextMessageDecorator.new(message).to_h).strip + message_format = config.message_format(self.class.name) + message_data = TextMessageDecorator.new(message).to_h + (message_format % message_data).strip end end end diff --git a/lib/pronto/formatter/text_message_decorator.rb b/lib/pronto/formatter/text_message_decorator.rb new file mode 100644 index 00000000..f97414c8 --- /dev/null +++ b/lib/pronto/formatter/text_message_decorator.rb @@ -0,0 +1,46 @@ +module Pronto + module Formatter + class TextMessageDecorator < SimpleDelegator + include Colorizable + + LOCATION_COLOR = :cyan + + LEVEL_COLORS = { + info: :yellow, + warning: :magenta, + error: :red, + fatal: :red + }.freeze + + def to_h + original = __getobj__.to_h + original[:color_level] = format_level(__getobj__) + original[:color_location] = format_location(__getobj__) + original + end + + private + + def format_location(message) + line = message.line + lineno = line.new_lineno if line + path = message.path + commit_sha = message.commit_sha + + if path || lineno + path = colorize(path, LOCATION_COLOR) if path + "#{path}:#{lineno}" + elsif commit_sha + colorize(commit_sha[0..6], LOCATION_COLOR) + end + end + + def format_level(message) + level = message.level + color = LEVEL_COLORS.fetch(level) + + colorize(level[0].upcase, color) + end + end + end +end