Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB #186

cllns · 2024-07-05T19:50:58Z

This is an experiment I worked on a couple weeks ago: to generate simple class/module definitions in Ruby, that nest properly in modules and without needing ERB templates. This is a stripped down version of that experiment, where we just pass-in body and headers instead of more specific args, based on excellent feedback from @timriley.

In this PR, I just convert the Operation generator (very recently merged in #171) to use the RubyFileGenerator. I wanted to give an bounded example that's easy to grok, rather than a huge messy diff. Plus I want to get feedback on this before putting in the work to change all the others.

Advantages:

Creating/modifying generators is less brittle. This work ensures the output is parseable. We can also push a lot of the inflection stuff into the RubyFileGenerator, instead of assuming that the name are properly formatted.
A big advantage of this is that we should be able to create a ComponentGenerator then have the OpereationGenerator inherit from that and just add a parent class, and that's it. In general, much of this (the modules based on app/slice) can be re-used for other generators, like Action as well
Enables some more powerful generators in the future, maybe something like hanami generate operation --deps repos.book_repo to automatically add include Deps["repos.book_repo"] etc.
Removes 'Context' classes, which were for ERB. Now all the logic is in the generator itself.

A disadvantage is that this adds more code, for now. I'm confident we'll be able to make up for that if we convert all the other files to use this, though and have this be a net-negative lines of code change, since we can get rid of the ERB templates and the 'Context' classes

I know we're pushing to get 2.2 out the door soon and this is refactoring that doesn't move the needle forward on that, so I'm happy to wait on this until after 2.2 is released and we start work on 2.3 :)

By adding new templates for un-nested operations

This reverts commit a5bd2f3.

Add dry-monads include for slice base action

timriley

Brilliant, @cllns! This is going to be a huge win for our generators. 😍😍😍

I've left a couple of thoughts for you. The only one worth doing before merging would be rearranging a couple methods, with the others being ideas for future refactors.

timriley · 2024-07-14T00:42:40Z

lib/hanami/cli/commands/app/generate/command.rb

+            def initialize(
+              fs:,
+              inflector:,
+              generator_class: nil,


I feel like generator_class is better suited to being a class attribute at this point.

Now that we're asking for a class for the generator, and taking care of initializing it with our standard options (fs, inflector, out), the generator itself no longer needs to be injectable for the purposes of testing, and beyond that there's really no need to be able to switch the generator at all.

This way, the subclasses of CLI::Commands::App::Generate can just specify their generator class inter class body, and not even have to worry about overriding #initialize.

Note: I think this would be fine to experiment with in a follow-up PR rather than blocking this one any further :)

Already addressed in #199, I forgot to backport it here :)

timriley · 2024-07-14T00:47:50Z

lib/hanami/cli/commands/app/generate/operation.rb

-            argument :name, required: true, desc: "Operation name"
-            option :slice, required: false, desc: "Slice name"
-
+          class Operation < Generate::Command


I love how tidy this class has become!

timriley · 2024-07-14T00:50:23Z

lib/hanami/cli/generators/app/operation.rb

+            base = if slice
+                     fs.join("slices", slice)
+                   else
+                     fs.join("app")
+                   end


If we can pass an actual slice instance into this, we could use our new Slice#source_path to make this simpler:

base = slice.source_path

This returns the appropriate value for both the app as well as slices, so it means we no longer need if app else slice conditionals everywhere, which has been a bother for the longest time :)

(Again, this would be another great refactor for post-merge, I don't want to hold anything up!)

Extracted to #202

timriley · 2024-07-14T00:54:04Z

lib/hanami/cli/ruby_file_generator.rb

+      def self.class(class_name, **args)
+        new(class_name: class_name, **args).to_s
+      end
+
+      def self.module(*names, **args)
+        module_names = if names.first.is_a?(Array)
+                         names.first
+                       else
+                         names
+                       end
+
+        new(modules: module_names, class_name: nil, parent_class: nil, **args).to_s
+      end


Minor nit, could these class methods go above #initialize? Having class methods up top is roughly how we've been arranging our code in general.

timriley · 2024-07-14T00:58:20Z

lib/hanami/cli/ruby_file_generator.rb

+      def lines(remaining_modules)
+        this_module, *rest_modules = remaining_modules
+        if this_module
+          with_module_lines(this_module, lines(rest_modules))


Recursion! Drink!

(Very clever solution, nice work 😄)

Hahah, I think that I knew this could be done recursively to arbitrary depth was one of the reasons I was excited to write this file.

timriley · 2024-07-14T00:58:59Z

lib/hanami/cli/ruby_file_generator.rb

+
+module Hanami
+  module CLI
+    class RubyFileGenerator


Love it. This class is easy to follow and is so powerful in how much it will simplify the rest of the generators.

Yea, I'm surprised I wasn't able to find any prior work for creating a Ruby file in Ruby. It was pretty fun to write though, so I don't mind :D

This reverts commit 303f502. Would need to namespace it and we may want to this to standalone so keeping it here. It's just two little spaces anyway

* Add struct generator * Extract App::Generate::Command * Specify full name, to use App::Command * Use App::Generate::Command for Struct * Use KEY_SEPARATOR from constants file * Provide generator class to command * Extract Helper, use for Struct generator * Fix specs with Helper * kwargs * Rename helper to RubyFileWriter * Fix examples * Remove optional kwarg * Reorder args * Rename to relative_parent_class * Remove duplicate implementation * Add api doc comments * Reorder methods * Refactor initialize * Refactor to use method instead of arg * Refactor to move logic into generator * Reorder assignments * Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB (#186) * Add dry-operation to default Gemfile * Add base Operation class, based on dry-operation * Fix view spec * Add Operation generators * Add empty `call` method definition * Remove ostruct * Allow slash separator for generator * Allow slash separator for generator * Rename module to admin * Remove newlines in generated files By adding new templates for un-nested operations * Remove input as default args * Remove Operations namespace, generate in app/ or slices/SLICE_NAME/ * Prevent generating operation without namespace * Revert "Prevent generating operation without namespace" This reverts commit a5bd2f3. * Add recommendation to add namespace to operations * Change examples * Switch to outputting directly, remove Files#recommend * Add Hanami::CLI::RubyFileGenerator * x.x.x => 2.2.0 * x.x.x => 2.2.0 * Include Dry::Monads[:result] in base Action * Add .module tests * Convert top-level app operation to use RubyFileGenerator * Convert nested app operation to use RubyFileGenerator * Support slash separators * Convert top-level slice operation to use RubyFileGenerator * Remove OperationContext * Remove namespaces instance variable * Refactor to variables * Remove last temporary instance variable * Refactor * More refactoring, for clarity * Rename variable for clarity * Rename helper method * Simplify RubyFileGenerator, support older versions * Convert Operation generator to use simplified RubyFileGenerator * Remove un-used errors * Refactor * Older kwargs forwarding style * Refactor * Rename variable * Add explanatory comment Add dry-monads include for slice base action * Fix base slice action * Remove un-used ERB templates * Remove OperationContext * Ternary over and/or * Fix missing 'end' from bad merge * Fix namespace recommendation * Extract App::Generate::Command * Specify full name, to use App::Command * Use constants file * Move class methods above initialize * Use constants file * Add yard comments * Revert "Use constants file" This reverts commit 303f502. Would need to namespace it and we may want to this to standalone so keeping it here. It's just two little spaces anyway * Fix indent to be two spaces * Remove extraneous requires * Use out.string.chomp * Fix name of expectation

* Add dry-operation to default Gemfile * Add base Operation class, based on dry-operation * Fix view spec * Add Operation generators * Add empty `call` method definition * Remove ostruct * Allow slash separator for generator * Allow slash separator for generator * Rename module to admin * Remove newlines in generated files By adding new templates for un-nested operations * Remove input as default args * Remove Operations namespace, generate in app/ or slices/SLICE_NAME/ * Prevent generating operation without namespace * Revert "Prevent generating operation without namespace" This reverts commit a5bd2f3. * Add recommendation to add namespace to operations * Change examples * Switch to outputting directly, remove Files#recommend * Add Hanami::CLI::RubyFileGenerator * x.x.x => 2.2.0 * x.x.x => 2.2.0 * Include Dry::Monads[:result] in base Action * Add .module tests * Convert top-level app operation to use RubyFileGenerator * Convert nested app operation to use RubyFileGenerator * Support slash separators * Convert top-level slice operation to use RubyFileGenerator * Remove OperationContext * Remove namespaces instance variable * Refactor to variables * Remove last temporary instance variable * Refactor * More refactoring, for clarity * Rename variable for clarity * Rename helper method * Simplify RubyFileGenerator, support older versions * Convert Operation generator to use simplified RubyFileGenerator * Remove un-used errors * Refactor * Older kwargs forwarding style * Refactor * Rename variable * Add explanatory comment Add dry-monads include for slice base action * Fix base slice action * Remove un-used ERB templates * Remove OperationContext * Ternary over and/or * Fix missing 'end' from bad merge * Fix namespace recommendation * Extract App::Generate::Command * Specify full name, to use App::Command * Use constants file * Move class methods above initialize * Use constants file * Add yard comments * Revert "Use constants file" This reverts commit 303f502. Would need to namespace it and we may want to this to standalone so keeping it here. It's just two little spaces anyway * Fix indent to be two spaces * Generate struct (with RubyFileGenerator, add RubyFileWriter) (#199) * Add struct generator * Extract App::Generate::Command * Specify full name, to use App::Command * Use App::Generate::Command for Struct * Use KEY_SEPARATOR from constants file * Provide generator class to command * Extract Helper, use for Struct generator * Fix specs with Helper * kwargs * Rename helper to RubyFileWriter * Fix examples * Remove optional kwarg * Reorder args * Rename to relative_parent_class * Remove duplicate implementation * Add api doc comments * Reorder methods * Refactor initialize * Refactor to use method instead of arg * Refactor to move logic into generator * Reorder assignments * Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB (#186) * Add dry-operation to default Gemfile * Add base Operation class, based on dry-operation * Fix view spec * Add Operation generators * Add empty `call` method definition * Remove ostruct * Allow slash separator for generator * Allow slash separator for generator * Rename module to admin * Remove newlines in generated files By adding new templates for un-nested operations * Remove input as default args * Remove Operations namespace, generate in app/ or slices/SLICE_NAME/ * Prevent generating operation without namespace * Revert "Prevent generating operation without namespace" This reverts commit a5bd2f3. * Add recommendation to add namespace to operations * Change examples * Switch to outputting directly, remove Files#recommend * Add Hanami::CLI::RubyFileGenerator * x.x.x => 2.2.0 * x.x.x => 2.2.0 * Include Dry::Monads[:result] in base Action * Add .module tests * Convert top-level app operation to use RubyFileGenerator * Convert nested app operation to use RubyFileGenerator * Support slash separators * Convert top-level slice operation to use RubyFileGenerator * Remove OperationContext * Remove namespaces instance variable * Refactor to variables * Remove last temporary instance variable * Refactor * More refactoring, for clarity * Rename variable for clarity * Rename helper method * Simplify RubyFileGenerator, support older versions * Convert Operation generator to use simplified RubyFileGenerator * Remove un-used errors * Refactor * Older kwargs forwarding style * Refactor * Rename variable * Add explanatory comment Add dry-monads include for slice base action * Fix base slice action * Remove un-used ERB templates * Remove OperationContext * Ternary over and/or * Fix missing 'end' from bad merge * Fix namespace recommendation * Extract App::Generate::Command * Specify full name, to use App::Command * Use constants file * Move class methods above initialize * Use constants file * Add yard comments * Revert "Use constants file" This reverts commit 303f502. Would need to namespace it and we may want to this to standalone so keeping it here. It's just two little spaces anyway * Fix indent to be two spaces * Remove extraneous requires * Use out.string.chomp * Fix name of expectation

cllns added 30 commits June 14, 2024 16:01

Add dry-operation to default Gemfile

d3e5e78

Add base Operation class, based on dry-operation

888c388

Fix view spec

24d3d6b

Add Operation generators

bc2670a

Add empty call method definition

1c37df3

Remove ostruct

a3f6a03

Merge branch 'main' into add-dry-operation

d93dfc8

Allow slash separator for generator

649bdcb

Allow slash separator for generator

f74519f

Rename module to admin

0f9f814

Remove newlines in generated files

663abc6

By adding new templates for un-nested operations

Remove input as default args

3b72feb

Remove Operations namespace, generate in app/ or slices/SLICE_NAME/

0f81a5c

Prevent generating operation without namespace

a5bd2f3

Revert "Prevent generating operation without namespace"

eb391ca

This reverts commit a5bd2f3.

Add recommendation to add namespace to operations

1023225

Change examples

6a3c32a

Switch to outputting directly, remove Files#recommend

8dc3de4

Add Hanami::CLI::RubyFileGenerator

7059e7e

x.x.x => 2.2.0

0dfdf7d

x.x.x => 2.2.0

8f90b33

Include Dry::Monads[:result] in base Action

8e62aa3

Add .module tests

f0e0994

Convert top-level app operation to use RubyFileGenerator

ed47e5e

Convert nested app operation to use RubyFileGenerator

4769bd6

Support slash separators

8e5c575

Convert top-level slice operation to use RubyFileGenerator

8effa30

Remove OperationContext

06f2750

Remove namespaces instance variable

79699f0

Refactor to variables

3879144

cllns added 17 commits June 24, 2024 17:24

Refactor

0223bea

Older kwargs forwarding style

8e458ce

Refactor

d4e13bd

Rename variable

f9221f0

Add explanatory comment

5f5887b

Add dry-monads include for slice base action

Merge remote-tracking branch 'origin/main' into add-file-generator

e9e7051

Fix base slice action

a051c7d

Remove un-used ERB templates

d0d02c7

Remove OperationContext

145f7ee

Ternary over and/or

b76942c

Fix missing 'end' from bad merge

26a665b

Fix namespace recommendation

b07cf35

Merge remote-tracking branch 'origin/main' into add-file-generator

23aace2

Extract App::Generate::Command

f649975

Specify full name, to use App::Command

5239824

Merge remote-tracking branch 'origin' into add-file-generator

511b6a0

Use constants file

fffd123

cllns marked this pull request as ready for review July 12, 2024 20:10

cllns changed the title ~~[PoC] Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB~~ Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB Jul 12, 2024

cllns mentioned this pull request Jul 13, 2024

Generate struct (with RubyFileGenerator, add RubyFileWriter) #199

Merged

timriley approved these changes Jul 14, 2024

View reviewed changes

cllns added 5 commits July 13, 2024 19:12

Move class methods above initialize

54dd28b

Use constants file

303f502

Add yard comments

e824e5d

Revert "Use constants file"

94c92e1

This reverts commit 303f502. Would need to namespace it and we may want to this to standalone so keeping it here. It's just two little spaces anyway

Fix indent to be two spaces

66d0c80

cllns mentioned this pull request Jul 14, 2024

Use slice.source_path in RubyFileWriter #202

Open

cllns merged commit d8837ec into main Jul 14, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB #186

Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB #186

cllns commented Jul 5, 2024

timriley left a comment

timriley Jul 14, 2024

cllns Jul 14, 2024

timriley Jul 14, 2024

timriley Jul 14, 2024

cllns Jul 14, 2024

timriley Jul 14, 2024

cllns Jul 14, 2024

timriley Jul 14, 2024

cllns Jul 14, 2024

timriley Jul 14, 2024

cllns Jul 14, 2024

Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB #186

Add Hanami::CLI::RubyFileGenerator, convert Operation to use it instead of ERB #186

Conversation

cllns commented Jul 5, 2024

timriley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment