Plugin module name and file names do not match RubyGems convention

[e2]

expected: Guard::LiveReload and lib/guard/live_reload.rb
or
expected: Guard::Livereload and lib/guard/livereload.rb (suggested/recommended)

actual: Guard::LiveReload and lib/guard/livereload.rb

see: http://guides.rubygems.org/name-your-gem/

(How Guard handles this currently is irrelevant, since it should be fixed to follow this convention as well).

[andreyvit]

Hey! Personally, I think that ‘livereload’ is such a widespread way of naming LiveReload-related things and files that inserting a dash or an underscore there will look weird. (But I'm not a maintainer or even a contributor of this project, so I have no say in the decision here.)

[e2]

@andreyvit - thanks for the comment!

I agree (I'm switching recommendations) - I doubt people will be using the plugin's API directly, so there shouldn't be any harm in switching to Guard::Livereload (so the filenames can stay as 'livereload').

Of course, this means bumping the major version number (though I doubt anyone would be affected anyway).

[thibaudgg]

👍 for Guard::Livereload!

[andreyvit]

Livereload makes me cringe, even more so than live_reload. Does this really need to be fixed? :-) And yet we have it easy — imagine naming files for a gem like Is It iPhone.

[e2]

TD;DR - yes, it's absolutely worth fixing, because ... reasons! :)

Summary:

• it breaks the convention
• forces other plugin authors to break the convention for "Guard to work"
• confusing code in Guard
• confusing/vague errors from Guard (since it can't guess whether LiveReload or LiveReload or Live::Reload or Li::VeReLoAd is missing)
• LiveReload is the "odd man out here"
• too easy to break one plugin while fixing another
• it's a pure waste of time when something doesn't "match"
• complexity of Guard code causes explosion of unit tests for proper coverage
• properly unit testing code related to mapping names, enumerating constants, simulating gem sets, requiring files requires some mad metaprogramming skills (and often needs patches for RSpec)
• the plugin managing code in Guard is tough to refactor as it is (without breaking semver, etc.)
• as I'm preparing for Guard 3.x, it's the best time to "clean" up (and this mean plugins too)
• some plugin authors have already paid a severe price (life/time is precious) because of the confusion and changes related to naming

... and besides: what's the gain of NOT making Guard::LiveReload match the Rubygem convention?

@andreyvit - if there's a Rubygems convention, I think it's worth aiming for. Otherwise, it's too easy to break things for people who are following the convention.

Livereload makes me cringe

It only affects Guard::Live[Rr]eload plugin maintainers - end users won't see it. I'm also not asking to change the gem name (which should according to the convention be guard-live_reload). (If you really want to see something cringe-worthy, check out parts in the Guard codebase for handling this - and the specs. It's not that the code is poorly written, btw.)

even more so than live_reload

Changing the module name has less impact and matches the convention without breaking much.
Here's what I'm considering:

1. gem name (guard_livereload suggests Guard::Livereload is the plugin constant)
2. template path (lib/guard/livereload/templates/Guardfile which Guard currently "incorrectly" infers from the module name)
3. guard name (:livereload, which Guard "looks up" within "normalized" existing names)
4. require path (which Guard "incorrectly" infers from names with dashes, forcing plugins to break the Rubygems convention)

Does this really need to be fixed? :-)

I'll just say the current Guard "name mapping" implementation is confusing, breaks the convention, isn't properly documented for plugin authors (various name mapping details spread throughout the Guard project) ... and I've already spent a few hours just debugging stuff (instead of fixing real issues or implementing new features).

Example: I first tried to fix guard-jekyll-plus (note the name!) and guard-ctags-bundler and guard-rails-assets and guard-livereload - and since I wasn't doing it all in one day, I didn't immediately realize I was fixing one gem while breaking support for another.

(NOTE: most guard plugins don't have an automated way of acceptance-testing template generation - which is why breaking things is so easy, hard to detect and confusing)

Pure waste of time - simply because of the approach "LiveReload seems nicer, so let's support it as a special case". (I'd much prefer an "ugly" module name and getting a NameError instead of a "confusing failure" because of strange way of mapping names - full of legacy "workarounds").

It's not like Guard::LiveReload has thousands of files and hundreds of API consumers.

I get that even popular gems like RuboCop break the RubyGems convention - the difference is RuboCop is required by human beings and guard plugins are loaded/configured/managed by Guard, so it makes sense to be more strict and pedantic about things.

Also, if the convention was strictly followed, users (and plugin authors) could get less confusing errors from guard.

Anyway, since I'm aiming to dump as much legacy support as possible in Guard 3.x, this is the best time to fix this.

[e2]

So ... any objections to switching to Guard::Livereload as the least disruptive option?

[andreyvit]

So let me answer this in 3 ways.

First, I'm not in a position to raise objections, and the actual maintainer here has said 👍 for Livereload.

Second, I didn't realize Guard had to special-case the name internally. Obviously we don't want that.

Third, I personally give a big priority to how something looks, and believe that if something gives rise to ugliness, it's broken and should be fixed. So I would personally pick a different rule for Guard to find plugins (something like “we normalize all class names by converting to lowercase, then compare to the guard name with underscores removed; if they match, it's the plugin we want”). However, given the particular rule Guard uses right now, I agree that Livereload is about all we can do.

Finally, to avoid the code using an ugly name everywhere, I would suggest keeping LiveReload and then assigning it to Livereload.

[andreyvit]

Okay, I see that Guard already has that loose matching that I mentioned here, and you want to get rid of it. Well... I would then replace it with an optional registration code, so that if someone doesn't like the convention, they can register a specific class for a given guard. There are camel-case names out there, we shouldn't pretend like they don't exist.

But yes, the only benefit is purely visual, so if you can define a class you want and then do Livereload = LiveReload (I don't actually remember if it's possible in Ruby), then cool.

[e2]

TL;DR - normalizing names seems "convenient", but it creates ambiguity, which should be handled/resolved, which means more code and tests in Guard

First, I'm not in a position to raise objections

I appreciate your feedback (and objections), so I took time to respond.

Second, I didn't realize Guard had to special-case the name internally.

Guard accumulated a lot of "hacks and patches" to fix immediate problems - which was a good approach ("getting things to just work for now"), because it was part of the learning process - and most of the issues surfaced only in hindsight. Plugin authors patched their stuff, and then Guard was patched to handle those patches and still be backward-compatible.

Too much code like that and suddenly major bugfixes require major release to avoid breaking semver.

Third, I personally give a big priority to how something looks, and believe that if something gives rise to ugliness, it's broken and should be fixed.

You can apply that to Guard's handling of the naming more so than to the Livereload const.

Also, in this case, this would mean fixing the gem name - or I e.g. don't understand why Livereload is "ugly" and guard-livereload is "nice" (as opposed to "guard-live_reload").

I 100% agree "not nice" suggests something is broken. Congruently though - doesn't the gem name bother you more? Either we're separating words in a name, or not - otherwise, it's a "double standard".

So I would personally pick a different rule for Guard to find plugins (something like “we normalize all class names by converting to lowercase, then compare to the guard name with underscores removed; if they match, it's the plugin we want”).

The problem with "normalizing" too aggressively, is that you can too easily create ambiguity, e.g. I can no longer author a new plugin Guard::LivereLoad (say related to a project called 'Livere'), even if my Guardfile has: guard :livere_load, even though the namespace (and plugin) has nothing in common with Guard::LiveReload. And I especially wouldn't be able to use both plugins, since the required file would be 'livereload' in both cases.

However, given the particular rule Guard uses right now, I agree that Livereload is about all we can do.

I don't care as much about the existing rules in Guard, since they can be moved to a "compatibility layer" (and gradually deprecated as conflicts and issues arise). If RubyGems has a convention that prevents ambiguity, that's a very strong selling point for me.

Bundle can require the file, because it doesn't process the module name - but Guard does rely on the module name, because it has no idea which files were required by Bundler (maybe this suggests a design flaw in Guard somewhere - and your question about the need for fixing may be completely justified).

As a minor naming issue, Guard::LiveReload version is in Guard::LiveReloadVersion (and it's inside 'version.rb'), which e.g. down the road would prevent guard from possibly keeping versions of generated templates and updating them if necessary. Unless we make Guard "messy" by also checking with e.g.

def version
  plugin_class.const_get(:VERSION)
rescue NameError
  Guard.const_get("#{plugin_class}Version")
end

which would only add unnecessary complexity and unit tests to Guard. Which shows that although Guard could just "make things work", practically it just means more crazy metaprogramming work in Guard (instead of renaming files and variables in plugins) - work no one really is eager to do.

Finally, to avoid the code using an ugly name everywhere, I would suggest keeping LiveReload and then assigning it to Livereload.

That generally is a good idea, as long as you don't rely on :to_s (which Guard does), because of:

module LiveReload # keep original everywhere
  # (...)
end

Livereload = LiveReload # provide alternative name

Livereload.to_s # what Guard does, and result: => "LiveReload" (surprise!)

[e2]

TL; DR - registration code won't work, and CamelCase would be handled by simply using underscores in the guard name.

Okay, I see that Guard already has that loose matching that I mentioned here, and you want to get rid of it.

Yes - refactoring and deprecating old stuff.

Well... I would then replace it with an optional registration code, so that if someone doesn't like the convention, they can register a specific class for a given guard.

"Chicken and egg" problem - you can't register a class without first requiring a file with the class definition or initializer (which would call the registration).

And it doesn't make sense to define that class in the Guardfile, because it is generated by instantiated plugins, anyway. (Unless you expect people to require gems manually in their Guardfiles - and in that case Guard shouldn't require files itself).

So, a guard name has to resolve to a gem name (or existing Guard::Plugin subclass in the Guard module namespace).

There are camel-case names out there, we shouldn't pretend like they don't exist.

That's what the compatibility layer would handle. The key feature of the compatibility layer would be resolving ambiguities and better error messages/debugging - without poluting the core Guard functionality with "new+legacy code".

I'm not trying to eliminate camel-case names. On the contrary - I want to make them obvious from the name.

E.g:

# unambigous
guard :railsassets # means Guard::Railsassets in 'lib/guard/railsassets.rb' (gem: guard-railsassets)
guard :rails_assets # means Guard::RailsAssets in 'lib/guard/rails_assets.rb' (gem: guard-rails_assets)
guard :'rails-assets' # would mean Guard::Rails::Assets (not supported yet by Guard - and may never be) in 'lib/guard/rails/assets.rb' (gem: guard-rails-assets)

# for compatibility
guard :rubocop # expects Guard::Rubocop, finds Guard::RuboCop, shows deprecation about using 'guard :rubo_cop' instead

But yes, the only benefit is purely visual, so if you can define a class you want and then do Livereload = LiveReload

That creates ambiguity (and non-deterministic given the order AFAIK - so sometimes guard init will work, sometimes it won't).

Guard treats dashes and underscores the same - as class name "word separators". (This prevents using e.g. Guard::MyPluginNamespace::MyPlugin).

[e2]

@andreyvit - I appreciate the discussion very much. It helped me realize a few important things.

If it helps, Guard needs to make 2 conversions:

1. Guardfile evalutation: guard name -> plugin class (and to gem to require it if necessary)
2. Initializing templates: plugin class -> plugin full path and plugin template path

The RubyGems convention would help simplify handling of both.

Since only Guard itself will ever "see" the actual plugin class name, I believe there is practically no "visual benefit" in this context. Guard::Livereload is unlikely to start growing features, and every file already has the namespace - so it's unlikely anyone will actually visually notice the change in reality.

[andreyvit]

Okay, pragmatism wins any day, so consider myself settled with Livereload. (I have just cringed as promised.)

The reason I don't like live_reload is because I consider livereload to be the only proper lowercase spelling of LiveReload. I would never e.g. call a web site live-reload.com, and would never spell it “Live Reload”. (To me, there is a third category between a “single word” and “separate words”, and that's a “single word composed of several camel-case components”.) And I'm generally appaled by a requirement to capitalize in a certain way in general, because so many things (like iPhone and TeX) require a specific non-standard capitalization.

It's true that there's zero practical difference and no impact on the user base. It's just about the internal craftsmanship.

As for Guard, I don't see why it's such a big deal. How about this?

module Guard
    class Plugin
        def self.guard_name
            self.to_s.underscore
        end

        def self.inherited(subclass)
            (@plugins ||= []) << subclass
        end

        def self.plugin_named(name)
            (@plugins || []).find { |p| p.guard_name == name }
        end
    end
end

class SomePlugin < Guard::Plugin
end

class LiveReload < Guard::Plugin
    def self.guard_name
        'livereload'
    end
end

puts SomePlugin.guard_name                      # => some_plugin
puts LiveReload.guard_name                      # => livereload
puts Guard::Plugin.plugin_named('some_plugin')  # => SomePlugin
puts Guard::Plugin.plugin_named('livereload')   # => LiveReload

[andreyvit]

"Chicken and egg" problem - you can't register a class without first requiring a file with the class definition or initializer (which would call the registration).

Unfortunately, I don't remember enough about Ruby module loading to propose a solution, but doesn't requiring a gem implicitly load all classes from it? I.e. when you see guard 'livereload', don't you simply do a require 'guard-livereload', and that makes sure everything is loaded?

Maybe I've spent too much time writing in Node where this stuff works much simpler.

[e2]

TL;DR - see: guard/guard#721

Okay, pragmatism wins any day, so consider myself settled with Livereload. (I have just cringed as promised.)

You give up too easily. ;)

The fact that we have to change a module name suggests Guard is translating between strings, filenames and modules - instead of using pure Ruby and methods.

I consider livereload to be the only proper lowercase spelling of LiveReload.

I don't know why you consider 'livereload' even correct. Technically, it's more of a filesystem/typing workaround.

If LiveReload is the correct name, it's the correct name - period.

I would never e.g. call a web site live-reload.com, and would never spell it “Live Reload”.

Also a workaround, and not correct. All the links to the page should be http://LiveReload.com. (what browser and search engines do with that name is neither in your control nor your concern).

(To me, there is a third category between a “single word” and “separate words”, and that's a “single word composed of several camel-case components”).

Some of those categories don't exist in a given domain (e.g. filenames on VFAT) - so every domain needs a representation that matches that domain (e.g. I can't have a '/' in my gem name - even if that's "correct" - and expect it to be accurately represented in every other domain).

And I'm generally appaled by a requirement to capitalize in a certain way in general, because so many things (like iPhone and TeX) require a specific non-standard capitalization.

Namespaces are more so for the interpreter to resolve ambiguity between symbol names. There's no "behavior" related to a module name. And in the code base you want to accurately name things based on what they do - not based on the actual title.

E.g. I can't have an iPhone class or module in Ruby, because that would be an error. But, that class can have a :to_s method that returns 'iPhone'.

So the following seems to be the "correct solution":

module Guard
  class Livereload < Guard::Plugin # fits the domain (RubyGems naming convention)
   def self.to_s
     "LiveReload" # fits the real tool name, can contain any characters in any encoding (though also somewhat restricted by domain, depending on what it's used for"
   end
  end
end

It's just about the internal craftsmanship.

That's why I spent so much thought on this - because it's about aiming for a resolution the "conflict", not trading "craftsmanship" for "convenient local pragmatism".

As for Guard, I don't see why it's such a big deal. How about this?

Class variables (need reseting in unit tests), lots of unit tests for coverage, memoization makes it more effort to test all cases, adding extra methods to Plugin API (future costs of backward compatibility), extra maintenance work in Guard, bumping version numbers, not DRY (since now both Guard and LiveReload share the responsibility of mapping), extra method in API, extra docs, technically requires a major (SemVer) release (just because other plugins could have implemented it for other purposes), etc.

Inheritance is more often than not a bad pattern in Ruby (BDD become horrible as a result).

Beside, it's additional code in the plugin, which seems to serve no "purpose" that can be tested in isolation. (Technically, every other plugin should now have unit tests for the guard_name).

Sure, it's 3 lines of code in the plugin implementation, but ... everything else is horrible (given we're just changing a single string).

A good step in the right direction though (in terms of ideas).

I.e. when you see guard 'livereload', don't you simply do a require 'guard-livereload', and that makes sure everything is loaded?

Guard first tries to see if the module is loaded, then it tries to load a file (in case people have require: false in the Gemfile, or they aren't using Bundler). And it gets messy when you find out that Guard uses regexen to find out if a plugin is used in a Guardfile...

solution

Notice that the above requires no "guard-specific" approach:

• LiveReload in the domain of case-insensitive filesystems (or URLs) would be 'livereload'.
• But in the RubyGems domain (here), LiveReload translates to live_reload

So, we can keep LiveReload (especially since the above makes the name actually useful and used), but ...

... we'd have to change the gem name to 'guard-live_reload' ("craftsmanship").

And, the naming code could be extracted into a generic gem (kind of like what ActiveSupport::Inflector does).

Thoughts?

[e2]

TL;DR - comparing Guard to Node tools

Maybe I've spent too much time writing in Node where this stuff works much simpler.

Assuming Bower+Grunt is to npm what Bundler+Guard is to gem...

Node is simpler mostly because:

• Bower copies stuff into the project (bower_components/...), while Guard looks up the template given the gem path (created from gem full path + transforming module name)
• Bower doesn't incrementally/conditionally generate/insert javascript code into your project like Guard does (see: Feature request: 'config.d' like approach for Guardfiles/templates guard#695 for an idea on how to fix this)
• it's up to the plugins to define available tasks in Grunt, while Guard plugins respond to a limited set of events (and uses config options instead of tasks) while providing hooks for customization

The potential advantage of Guard is that it can become smarter without being too overwhelming for casual users (mostly in terms of defining workflows). Grunt is more task-oriented - which is usually sufficient.

But there's a lot of cleanup to do first - or feature development will be slow.

[e2]

I'm closing this because after lots of thoughts I'm considering moving away from the DSL anyway:

guard/guard#796

This means instead of:

guard 'livereload'

it will be simply:

require 'guard/livereload'
livereload = Guard::LiveReload.new(options)
livereload.smart_reload(:public_assets)
livereload.full_reload(:rails_template_views)

or by using "default logic":

require 'guard/livereload'
Guard::LiveReload.setup(self, options, :defaults_with_rails)

(To avoid the require, it's best to rely on Bundler's autoloading - no more magic is necessary, really).

or even for now at least:

guard 'LiveReload' # simply uses Guard.const_get(:LiveReload)

There's no real reason to have the downcase string 'livereload' anywhere - because it doesn't serve any functional purpose.

Even the command line would be expected to be: guard init Guard::LiveReload

[e2]

Closing since shouldn't matter in Guard 3.x (once it's done).