Added --editor (-e) option to open generated files in the user's editor #32996

y-yagi · 2018-05-27T04:32:22Z

This is a retry of #8553.

Added support for generators added later from the original PR, and extract the function on the primary file into method.

Also avoied adding the editor option to Generators::Base.
If add it to Generators::Base, editor option will be output to the help of generator which does not support it. This confuses the user.

kaspth

Nice idea!

kaspth · 2018-07-02T17:24:09Z

railties/lib/rails/generators/primary_file_helpers.rb

+
+module Rails
+  module Generators
+    module PrimaryFileHelpers # :nodoc:


extend ActiveSupport::Concern?

kaspth · 2018-07-02T17:26:35Z

railties/lib/rails/generators/rails/controller/controller_generator.rb

-        template "controller.rb", File.join("app/controllers", class_path, "#{file_name}_controller.rb")
+        destination = File.join("app/controllers", class_path, "#{file_name}_controller.rb")
+        template "controller.rb", destination
+        primary_file(destination)


This obstructs the flow a bit too much for me.

How about adding a primary_template method? Or overriding template so we can pass a primary_file: true option?

Or perhaps primary_file can just return the destination. So we can do template "controller.rb", primary_file(File.join(…))

Is it possible to open multiple files in the editor too? It might be nice to open the whole shebang after a scaffold generator has run for instance.

Then it could make sense for template to collect files to open after the generator has finished.

That it depends on the editor whether or not can open multiple files (I'm not sure if it is possible with an editor without tab functions)
It is difficult to check the operation with various editors, so I would like to avoid supporting it.

template "controller.rb", primary_file(File.join(…)) seems nice. I will update.

My bad. In order to generate a file, need to run template first. So template "controller.rb", primary_file(File.join(…)) does not work.
Instead, I added the primary_template method.

kaspth · 2018-07-02T17:32:38Z

activerecord/lib/rails/generators/active_record/migration/migration_generator.rb

+
+        def primary_file?
+          shell.base.class.to_s == "Rails::Generators::MigrationGenerator"
+        end


I don't understand how these primary_file? checks work?

This generator will be triggered from Rails::Generators::MigrationGenerator, so this check works(Does this make the answer as expected...?).

I don't understand what you mean. Can you give an example of the complete code flow?

Perhaps it's just because I don't understand what shell or shell.base is here.

kaspth · 2018-07-02T17:34:55Z

railties/lib/rails/generators/primary_file_helpers.rb

+      def self.included(base)
+        base.class_option :editor, type: :string, aliases: "-e", lazy_default: ENV["EDITOR"],
+                                   required: false, banner: "editor",
+                                   desc: "Open generated primary file in the specified editor (Default: $EDITOR)"


What type of generators can't support the --editor option?

For example, it can not support scaffold generator which can not decide primary file.

This is a retry of rails#8553. Added support for generators added later from the original PR, and extract the function on the primary file into method. Also avoied adding the `editor` option to `Generators::Base`. If add it to `Generators::Base`, `editor` option will be output to the help of generator which does not support it. This confuses the user.

kaspth · 2018-07-21T11:39:15Z

railties/lib/rails/generators/primary_file_helpers.rb

+      included do
+        class_option :editor, type: :string, aliases: "-e", lazy_default: ENV["EDITOR"],
+                              required: false, banner: "editor",
+                              desc: "Open generated primary file in the specified editor (Default: $EDITOR)"


Should this really be an --editor option or a --edit option? Because we don't allow editor customization for the credentials:edit or secrets:edit commands, there $EDITOR is assumed.

I don't think we indent arguments like this. I'd just do:

class_option :editor, type: :string, … required: false, …

kaspth · 2018-07-21T11:39:55Z

railties/lib/rails/generators/primary_file_helpers.rb

+        end
+
+        def primary_file?
+          self.instance_of?(shell.base.class)


Could remove the self. here.

kaspth · 2018-07-21T11:40:45Z

railties/lib/rails/generators/primary_file_helpers.rb

+      included do
+        class_option :editor, type: :string, aliases: "-e", lazy_default: ENV["EDITOR"],
+                              required: false, banner: "editor",
+                              desc: "Open generated primary file in the specified editor (Default: $EDITOR)"


I don't think we indent arguments like this. I'd just do:

class_option :editor, type: :string, … required: false, …

kaspth · 2018-07-21T11:41:37Z

railties/lib/rails/generators/named_base.rb

+        def primary_template(source, destination)
+          template(source, destination)
+          primary_file(destination)
+        end


Think this makes more sense in the PrimaryFileHelpers module.

kaspth · 2018-07-21T11:47:21Z

railties/lib/rails/generators/primary_file_helpers.rb

+
+module Rails
+  module Generators
+    module PrimaryFileHelpers # :nodoc:


I'm curious about the naming choice here by the way. Why the word primary? Since there's only ever one file that's editable — and thus no secondary files — what makes it primary? Especially when this adds an editor option and not a primary_file option?

How about Rails::Generators::TemplateEditing which overrides template to support an edit: true option?

kaspth · 2018-07-21T11:47:41Z

railties/test/generators/migration_generator_test.rb

+        end
+      end
+    end
+  end


Dig all these tests 😍

kaspth · 2018-07-21T11:48:27Z

activerecord/lib/rails/generators/active_record/migration/migration_generator.rb

+
+        def primary_file?
+          shell.base.class.to_s == "Rails::Generators::MigrationGenerator"
+        end


Perhaps it's just because I don't understand what shell or shell.base is here.

kaspth · 2019-09-13T22:12:01Z

@y-yagi I just had a use for this, are you interested in picking it up again or do you want me or someone else to take over? It'd be really cool to have in 6.1 ✌️

y-yagi · 2019-09-14T00:54:28Z

Ya, I'm still interested in this. But I have another idea related to this. I'd like to think about what to do with this, so please wait ;)

kaspth · 2019-09-14T01:12:09Z

Sounds great! 😄

rails-bot · 2019-12-17T20:52:37Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

kaspth · 2019-12-22T22:25:25Z

@y-yagi did you get to attempt the other idea?

y-yagi · 2019-12-22T22:39:02Z

Sorry for the late action. I plan to provide a callback to perform processing on the generated file after the file is generated(The callback will support file operations in the editor and execution of rubocop, etc).

But still working on it. y-yagi@0470943 I am going to finish this in the new year holidays.

kaspth · 2019-12-22T23:03:07Z

Cool, interesting 😊

rails-bot · 2020-03-21T23:55:56Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

simi · 2020-03-29T10:04:46Z

@y-yagi this would be really helpful for me (and I was thinking about similar idea few times already) for generated migrations. I was looking at master...y-yagi:add_after_generate. Is there still any plan from your side for this? Is there anything I can help you with?

kaspth · 2020-03-29T13:43:14Z

@simi I think this has utility enough that 2 people can work on it — as long as you don’t erase credit. Base your work on the commit in that branch, get it rebased, flesh it out more and check allow edits from maintainers on the PR. Then @y-yagi will have an easier time wrapping it up if he has time to jump in or we can wrap it up if you make progress on it 👌

y-yagi · 2020-04-03T13:00:00Z

@simi Thanks for pulling up this and sorry for the late response.
I had finished that remaining fixed and created a PR #38870. I hope you can check if that helps your issue. Thanks!

simi · 2020-04-03T13:08:27Z

@y-yagi looking in there. Thanks for info.

y-yagi force-pushed the add_editor_option_to_generator branch 2 times, most recently from d7139f1 to 1294857 Compare June 9, 2018 00:17

y-yagi force-pushed the add_editor_option_to_generator branch 2 times, most recently from 20dd022 to 8900c4e Compare June 17, 2018 11:55

kaspth reviewed Jul 2, 2018

View reviewed changes

y-yagi force-pushed the add_editor_option_to_generator branch 2 times, most recently from f125234 to c422c20 Compare July 19, 2018 09:44

y-yagi force-pushed the add_editor_option_to_generator branch from c422c20 to d9118db Compare July 19, 2018 09:58

kaspth reviewed Jul 21, 2018

View reviewed changes

gmcgibbon added the railties label Sep 30, 2018

rails-bot bot added the stale label Dec 17, 2019

rails-bot bot removed the stale label Dec 22, 2019

rails-bot bot added the stale label Mar 21, 2020

rails-bot bot closed this Mar 29, 2020

simi mentioned this pull request Apr 3, 2020

Add config.generators.after_generate for processing to generated files #38870

Merged

Added --editor (-e) option to open generated files in the user's editor #32996

Added --editor (-e) option to open generated files in the user's editor #32996

Uh oh!

Conversation

y-yagi commented May 27, 2018

Uh oh!

kaspth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspth commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

y-yagi commented Sep 14, 2019

Uh oh!

kaspth commented Sep 14, 2019

Uh oh!

rails-bot bot commented Dec 17, 2019

Uh oh!

kaspth commented Dec 22, 2019

Uh oh!

y-yagi commented Dec 22, 2019

Uh oh!

kaspth commented Dec 22, 2019

Uh oh!

rails-bot bot commented Mar 21, 2020

Uh oh!

simi commented Mar 29, 2020

Uh oh!

kaspth commented Mar 29, 2020

Uh oh!

y-yagi commented Apr 3, 2020

Uh oh!

simi commented Apr 3, 2020

Uh oh!

Uh oh!

kaspth commented Sep 13, 2019 •

edited

Loading