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

Extract Guard::UI into a separate gem #713

Open
e2 opened this issue Jan 6, 2015 · 8 comments
Open

Extract Guard::UI into a separate gem #713

e2 opened this issue Jan 6, 2015 · 8 comments
Assignees
Labels
Milestone

Comments

@e2
Copy link
Contributor

e2 commented Jan 6, 2015

TL;DR - UI functionality should be in a separate gem to solve many problems

Basically to decouple the functionality from Guard, because it's a maintenance nightmare:

  • reliance on global state
  • dependency on other Guard:: modules (even though it's just for outputting in color)
  • mixing up logging with screen output
  • hard to refactor without breaking compatibility
  • hard to test because of global state / singleton
  • color handling is not transparent
  • too many responsibilities in one class (logging, option handling, coloring, line reset, deprecations, screen clearing, scope name, backtrace filtering, requiring win32console) while none are actually Guard-specific
  • options exposed as a hash makes little sense (logger options during init) and just makes the interface confusing
@e2 e2 added this to the v2.99.99 milestone Jan 6, 2015
@rymai
Copy link
Member

rymai commented Jan 6, 2015

👍

@e2
Copy link
Contributor Author

e2 commented Feb 22, 2015

Promoted to "bug", because guard-plugins depend on UI/notify functionality (but do not need anything else from guard) - and this causes problems for users.

@e2 e2 self-assigned this Feb 22, 2015
@ioquatix
Copy link
Member

What can we do to simplify this?

@ioquatix
Copy link
Member

ioquatix commented Jun 15, 2017

I thought about this.

There is quite a bit of code in guard for doing stuff like color, logging, etc, which is now better handled by standard logger, rainbow, with a custom formatter.

Why not deprecated all of the global state, deprecate guard-compat, and then inject that state into the classes directly.

e.g.

class Plugin
    def initialize(logger)
       @logger = logger
    end

    attr :logger
end

class MyPlugin < Plugin
end

We could add some compatibility hooks to make this as easy as possible. I think if you expose the global state, it's going to be an endless battle.

@ioquatix
Copy link
Member

We'd have to probably to a major version bump to implement this, but we could do it with minimal impact.

Perhaps we can tie it into the inevitable updates to listen.

@ioquatix ioquatix self-assigned this Jun 15, 2017
@ioquatix
Copy link
Member

ioquatix commented Jun 15, 2017

I've been working on https://github.com/socketry/guard-falcon and I've tried to figure out what a guard-plugin should look like.

Making the gem name a class e.g. class Guard::Falcon < Plugin is a small mistake that makes it hard to define Guard::Falcon::Version, so I've made (as is recommended by RubyGems) module Guard::Falcon with a method ::new which forwards/constructs Guard::Falcon::Controller. I guess it could be called Plugin too.

Ideally we'd just use local methods on the class, e.g. #logger as I've added, but this should be defined by Plugin.

We can do a fair amount of monkey patching to fix old plugins, including adding to the plugin base def Compat; self; end and alias UI logger which would allow a lot of existing code to continue working without any changes.

Thoughts?

@ioquatix
Copy link
Member

Another option is that we completely break away from the inheritance method of doing things, and use a module instead - e.g. Guard::Falcon is a model which defines start, restart, and so on, and can expect some standard things to be defined, e.g. attr :options. We then write something like Guard::Plugin.new.extend(Guard::Falcon). I think this was mentioned elsewhere. I'd be okay with a breaking change like this in a major version release, if the value of doing so was significant.

Ideally, we probably make a new gem, guard-plugin, which implements the very core plugin class, provides some testing harness, etc. Similar to guard-compat, but in a way that doesn't work based on the old assumptions, and allows users to incrementally update the plugins.

@rymai
Copy link
Member

rymai commented Jun 17, 2017

I think we should move away from the inheritance model, but I would argue that Guard should provide a module that plugin classes would include:

module Guard
  module API
    # A module with the content of the current Guard::Plugin
    # Guard::Plugin#initialize would show a deprecation warning.
  end
end

module Guard
  module RSpec
    class Plugin
      include ::Guard::API
    end
  end
end

The benefit is that Guard::RSpec would become a module.

See #873, guard/guard-compat#6, and guard/guard-rspec#397.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants