Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command title and description #167

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,11 @@ SketchupSuggestions/AddGroup:
Reference: https://github.com/SketchUp/rubocop-sketchup/tree/main/manual/cops_suggestions.md#addgroup
Enabled: true

SketchupSuggestions/CommandTitle:
Description: Use title case for command titles.
Reference: https://github.com/SketchUp/rubocop-sketchup/tree/main/manual/cops_suggestions.md#commandtitle
Enabled: true

SketchupSuggestions/Compatibility:
Description: Incompatible feature with target SketchUp version.
Reference: https://github.com/SketchUp/rubocop-sketchup/tree/main/manual/cops_suggestions.md#compatibility
Expand Down
44 changes: 44 additions & 0 deletions lib/rubocop/sketchup/cop/suggestions/command_title.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# frozen_string_literal: true

module RuboCop
module Cop
module SketchupSuggestions
# SketchUp command titles should be in title case, e.g. "Make Component"
# and "Follow Me".
#
# In English, capitalize the first letter of each word. Other languages
# may have different rules.
class CommandTitle < SketchUp::Cop

def_node_matcher :init_entity?, <<-PATTERN
(send (const (const nil? :UI) :Command ) :new ... )
PATTERN

MESSAGE = 'Use title case in command titles. '\
'In English, capitalize the first letter of each word.'

def on_send(node)
return unless init_entity?(node)
return if node.arguments.first.str_content.nil?
return if title_case?(node.arguments.first.str_content)

add_offense(node, message: MESSAGE)
end

private

# REVIEW: Extract to where they can be re-used?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe in a lib\rubocop\sketchup\formatting.rb file? A RuboCop::SketchUp::Formatting mixing module?
Like lib\rubocop\sketchup\namespace_checker.rb?

def title_case?(text)
text == title_case(text)
end

def title_case(text)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth a comment on how this differ from Ruby's own title case method.

text.gsub(/^(.)/) { ::Regexp.last_match(1).upcase }
.gsub(/\ (.)/) { " #{::Regexp.last_match(1).upcase}" }
.gsub(/-(.)/) { "-#{::Regexp.last_match(1).upcase}" }
.gsub(/(\.)$/, '')
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ In the following section you find all available cops:
#### Department [SketchupSuggestions](cops_suggestions.md)

* [SketchupSuggestions/AddGroup](cops_suggestions.md#sketchupsuggestionsaddgroup)
* [SketchupSuggestions/CommandTitle](cops_suggestions.md#sketchupsuggestionscommandtitle)
* [SketchupSuggestions/Compatibility](cops_suggestions.md#sketchupsuggestionscompatibility)
* [SketchupSuggestions/DynamicComponentInternals](cops_suggestions.md#sketchupsuggestionsdynamiccomponentinternals)
* [SketchupSuggestions/FileEncoding](cops_suggestions.md#sketchupsuggestionsfileencoding)
Expand Down
17 changes: 17 additions & 0 deletions manual/cops_suggestions.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ face2 = group.entities.add_face(points2)

* [https://github.com/SketchUp/rubocop-sketchup/tree/main/manual/cops_suggestions.md#addgroup](https://github.com/SketchUp/rubocop-sketchup/tree/main/manual/cops_suggestions.md#addgroup)

<a name='commandtitle'></a>
## SketchupSuggestions/CommandTitle

Enabled by default | Supports autocorrection
--- | ---
Enabled | No

SketchUp command titles should be in title case, e.g. "Make Component"
and "Follow Me".

In English, capitalize the first letter of each word. Other languages
may have different rules.

### References

* [https://github.com/SketchUp/rubocop-sketchup/tree/main/manual/cops_suggestions.md#commandtitle](https://github.com/SketchUp/rubocop-sketchup/tree/main/manual/cops_suggestions.md#commandtitle)

<a name='compatibility'></a>
## SketchupSuggestions/Compatibility

Expand Down
59 changes: 59 additions & 0 deletions spec/rubocop/cop/sketchup_suggestions/command_title_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# frozen_string_literal: true

require 'spec_helper'

describe RuboCop::Cop::SketchupSuggestions::CommandTitle do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this pass the tests we need to change this line to:

describe RuboCop::Cop::SketchupSuggestions::CommandTitle, :config do

And then remove:

subject(:cop) { described_class.new }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what was done to all our existing cops to fix the build:
679d2e2


subject(:cop) { described_class.new }

bad_capitalization = [
'text',
'text text',
'Text text',
'text Text',
'Text.',
'Text Text.',
'Text text.',
'text 2-Point',
'text 2-point'
]

bad_capitalization.each do |keyword|
it "registers an offense when using UI::Command.new(\"#{keyword}\")" do
expect_offense(<<~RUBY, keyword: keyword)
UI::Command.new("%{keyword}")
^^^^^^^^^^^^^^^^^^{keyword}^^ Use title case in command titles. [...]
RUBY
end

# TODO: Add for setter methods.
# Can we test by how a local variable was defined? Or only by its name?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you want to test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking something like one node catcher than looks for {x} = UI::Command.new(...) for any x, and then a second search in the same local scope for {x}.menu_text =. Otherwise it can maybe just check for menu_text = and assume there is no other method by the same name.

#
# command = UI::Command.new("Test")
# command.menu_text = "%{keyword}"
#
# menu.add_item("%{keyword}")
end

good_capitalization = [
'Text',
'Text Text',
'Text 2-Point',
'文本'
]

good_capitalization.each do |keyword|
it "does not register an offense when using UI::Command.new(\"#{keyword}\")" do
expect_no_offenses(<<~RUBY, keyword: keyword)
UI::Command.new("%{keyword}")
RUBY
end
end

it 'does not register an offense when using UI::Command.new(variable)' do
expect_no_offenses(<<~RUBY)
UI::Command.new(variable)
RUBY
end

end