davetron5000 / gli

Make awesome command-line applications the easy way
http://davetron5000.github.io/gli
Apache License 2.0
1.26k stars 103 forks source link

procedural vs oob definition for clean code architecture and file splitting #333

Closed noraj closed 3 months ago

noraj commented 3 months ago

In dry-cli, every command or sub-command is defined in a class inheriting Dry::CLI::Command (in an object-oriented programming manner), e.g. here, so it's very easy to split a large CLI code-base over several files and keep a clean code architecture. Then any command class is mapped to a CLI (sub-)command with the register command, e.g. like that.

I wanted to do the same splitting in gli, but it's harder since every command and sub-command is defined in a procedural programming manner. At most, I could split every top-level command in several files but for any sub-command, sub-sub-command and so on they can't be split as they are part of one block (a top-level command). So in a pretty nested CLI app with several sub-commands level this makes very long files and a pretty flat code architecture.

Could you give me advice of how could I order that better, if possible?

davetron5000 commented 3 months ago

See #331 for a few options. GLI was designed to act as it does, so having a design where a bunch of classes are inspected to determine the CLI UX is a pretty big change (though in retrospect it would be a better design). That said, to piggyback on the example from that issue:

# In e.g. lib/actions/duzdu
class DUZDUBaseAction
  def initialize(options)
    parent_options = options.dig(GLI::Command::PARENT, GLI::Command::PARENT)
    dns_opts = parent_options[:nameserver].nil? ? nil : { nameserver: [parent_options[:nameserver]] }
    @duzdu = ADAssault::DNS::DUZDU.new(parent_options[:domain], dns_opts)
  end

  def perform!
    raise "subclass must implement"
  end

  def self.to_proc
    ->(_global, options, args) {
      self.new(options)
    }
  end

  def self.cli_name = self.name.downcase

  def self.description
    raise "subclass must implement"
  end

  def self.define!(gli_command)
    gli_command.desc self.description
    gli_command.command self.cli_name do |c|
      c.action(&self)
    end
  end

end

class Add < DUZDUBaseAction
  def perform!
    @duzdu.addv4
  end
  def description = "Add a thing"
end

class Delete < DUZDUBaseAction
  def perform!
    @duzdu.deletev4
  end
  def description = "Delete a thing"
end

# then, in your GLI "app"

dns.command :duzdu do |duzdu|
  Add.define!(duzdu)
  Delete.define!(duzdu)
end

I think if I was starting over, and more OO and less DSL approach would be better ,but this is an old library and that was, as they say, the style at the time :)

noraj commented 3 months ago

It's true it's from 2009, thanks for the examples, advice and for still maintaining it.

davetron5000 commented 3 months ago

Thanks for using it! :)

noraj commented 3 months ago

Here's how I have architecture it: https://github.com/noraj/ADAssault/tree/0.0.2/lib/adassault/cli