Skip to content

Commit f5f0d60

Browse files
committed
Remove magic default value as per review comment
1 parent 9f1c8ba commit f5f0d60

File tree

3 files changed

+15
-24
lines changed

3 files changed

+15
-24
lines changed

README.md

+5-8
Original file line numberDiff line numberDiff line change
@@ -685,32 +685,29 @@ with Overcommit without writing any Ruby code in a similar way as
685685

686686
These special line-aware command hooks behave and are configured the same way
687687
as the Git ones, except only file arguments get passed to them.
688-
Also to enable the feature, they must use at least one of the following options,
689-
so that, using the command output:
688+
Also to enable them and for optimal use, one must configure them as explained
689+
below, so that, using the command output:
690690
- differentiating between warnings and errors becomes possible
691691
- modified lines can be detected and acted upon as defined by
692692
the `problem_on_unmodified_line`, `requires_files`, `include` and `exclude`
693693
[hook options](#hook-options)
694694

695695
**Warning**: Only the command's standard output stream is considered for now,
696696
*not* its standard error stream.
697-
If you do not need to change the default values for any other option,
698-
then the `extract_messages_from` option has to be specified.
699-
Its value does not matter for now, but it should be set to `stdout`
700-
to avoid problems in the future.
701697

702698
To differentiate between warning and error messages,
703699
the `warning_message_type_pattern` option may be specified:
704700
the `type` field of the `message_pattern` regexp below must then include
705701
the `warning_message_type_pattern` option's text.
706702

707703
The `message_pattern` option specifies the format of the command's messages.
708-
It is a optional [(Ruby) regexp][RubyRE], which if present must at least define
704+
It is mandatory, must be a [(Ruby) regexp][RubyRE], and must define at least
709705
a `file` [named capture group][RubyRENCG].
710706
The only other allowed ones are `line` and `type`, which when specified
711707
enable detection of modified lines and warnings respectively.
712708

713-
**Note**: The default value for this option is often adequate:
709+
**Tip**: The following value for this option is often adequate:\
710+
`!ruby/regexp /^(?<file>(?:\w:)?[^:]+):(?<line>\d+):[^ ]* (?<type>[^ ]+)/`:\
714711
it generalizes the quasi-standard [GNU/Emacs-style error format][GNUEerrf],
715712
adding the most frequently used warning syntax to it.
716713

lib/overcommit/hook_loader/base.rb

+2-9
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,8 @@ def load_hooks
2222

2323
private
2424

25-
# GNU/Emacs-style error format:
26-
AD_HOC_HOOK_DEFAULT_MESSAGE_PATTERN =
27-
/^(?<file>(?:\w:)?[^:]+):(?<line>\d+):[^ ]* (?<type>[^ ]+)/.freeze
28-
2925
def is_hook_line_aware(hook_config)
30-
hook_config['extract_messages_from'] ||
31-
hook_config['message_pattern'] ||
32-
hook_config['warning_message_type_pattern']
26+
hook_config['message_pattern']
3327
end
3428

3529
def create_line_aware_command_hook_class(hook_base) # rubocop:disable Metrics/MethodLength
@@ -53,8 +47,7 @@ def extract_messages(line_aware_config, result)
5347
warning_message_type_pattern = line_aware_config[:warning_message_type_pattern]
5448
Overcommit::Utils::MessagesUtils.extract_messages(
5549
result.stdout.split("\n"),
56-
line_aware_config[:message_pattern] ||
57-
AD_HOC_HOOK_DEFAULT_MESSAGE_PATTERN,
50+
line_aware_config[:message_pattern],
5851
Overcommit::Utils::MessagesUtils.create_type_categorizer(
5952
warning_message_type_pattern
6053
)

spec/overcommit/hook_loader/plugin_hook_loader_spec.rb

+8-7
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,18 @@
7878
flags:
7979
- "--format=emacs"
8080
include: '**/*.foo'
81-
FooLintDefault:
81+
FooLintRecommendedPattern:
8282
enabled: true
8383
command: ["foo", "lint"]
84+
message_pattern: !ruby/regexp /^(?<file>(?:\w:)?[^:]+):(?<line>\d+):[^ ]* (?<type>[^ ]+)/
8485
warning_message_type_pattern: warning
8586
flags:
8687
- "--format=emacs"
8788
include: '**/*.foo'
88-
FooLintDefaultNoWarnings:
89+
FooLintRecommendedPatternNoWarnings:
8990
enabled: true
9091
command: ["foo", "lint"]
91-
extract_messages_from: stdout
92+
message_pattern: !ruby/regexp /^(?<file>(?:\w:)?[^:]+):(?<line>\d+):[^ ]* (?<type>[^ ]+)/
9293
flags:
9394
- "--format=emacs"
9495
include: '**/*.foo'
@@ -143,8 +144,8 @@
143144
it { should fail_hook }
144145
end
145146

146-
describe '(using default pattern)' do
147-
let(:hook_name) { 'FooLintDefault' }
147+
describe '(using recommended pattern)' do
148+
let(:hook_name) { 'FooLintRecommendedPattern' }
148149

149150
context 'when command fails with some warning message' do
150151
let(:result) do
@@ -175,8 +176,8 @@
175176
end
176177
end
177178

178-
describe '(using defaults)' do
179-
let(:hook_name) { 'FooLintDefaultNoWarnings' }
179+
describe '(using recommended pattern w/o warnings)' do
180+
let(:hook_name) { 'FooLintRecommendedPatternNoWarnings' }
180181

181182
context 'when command fails with some messages' do
182183
let(:result) do

0 commit comments

Comments
 (0)