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

Fixes #19191 - Output formatter applies for selected adapter only #284

Conversation

ofedoren
Copy link
Member

Now it is possible to apply an output formatter on selected adapters only.

To use this feature, define applicable_adapters OR not_applicable_adapters method in your formatter like this:

class MyFormatter < HammerCLI::Output::Formatters::FieldFormatter
  def applicable_adapters
    [HammerCLI::Output::Adapter::Table]
  end

  def tags
    # tags
  end
  
  def format(data, field_params = {})
    # logic
  end
end

Similar to apply on all the adapters except a particular one:

class MyFormatter < HammerCLI::Output::Formatters::FieldFormatter
  def not_applicable_adapters
    [HammerCLI::Output::Adapter::Table]
  end

  def tags
    # tags
  end
  
  def format(data, field_params = {})
    # logic
  end
end

NOTE: If none of the methods defined, everything has default behaviour.

@mbacovsky
Copy link
Member

@ofedoren looks good! Could you cover this in tests?

Did you consider using adapter names (under which they are registered) - :table, :csv instead of the class name? This way the formatter filtering would still work if e.g. the table adapter implementation is replaced. Just an idea, not sure if that is a good thing yet.

@mbacovsky
Copy link
Member

@ofedoren I've also re-checked the original issue and think this approach duplicates the tags. I'll comment in the issue.

@ofedoren
Copy link
Member Author

ofedoren commented Feb 7, 2019

@mbacovsky, updated.

Now it uses tags for comparison.

@ofedoren
Copy link
Member Author

ofedoren commented Apr 9, 2019

blocked by #300

@mbacovsky
Copy link
Member

@ofedoren, the more I think about this the more I tend to keep just #300 and close this PR and the #19191 issue as won't fix or duplicate. The only use-case mentioned was fixed by 300 and I think it is better practice to add new "tags" then make formatters bound to specific adapters. Makes sense?

@ofedoren
Copy link
Member Author

@mbacovsky, I didn't even notice, that #300 fixed that use-case :) As shows that PR, we can add another formatter or "tags", so I'll close this PR with the issue as duplicate.

@ofedoren ofedoren closed this Apr 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants