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

Updated #5829: Added --editor (-e) option to open all generated & copied files in the user's editor #8553

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@ndbroadbent
Contributor

ndbroadbent commented Dec 18, 2012

This is an updated version of #5829, with @rafaelfranca's proposal to make it higher level.

The --editor option is now aliased as -e, and is available for all generators that inherit from Rails::Generators::Base. The default editor is now GUI_EDITOR, and falls back to EDITOR.

It wraps Thor's template and copy_file actions, and opens generated/copied files in your text editor if options["editor"] is present.

P.S. I initially tried to just open the main generated files, but I think it's better to just open them all. If you run the model generator, you'll probably want to edit the migration, model, and tests. (Instead of just the model.)

@frodsan

View changes

Show outdated Hide outdated railties/lib/rails/generators/actions.rb
@frodsan

View changes

Show outdated Hide outdated railties/lib/rails/generators/actions.rb
@frodsan

View changes

Show outdated Hide outdated railties/lib/rails/generators/actions.rb
@frodsan

View changes

Show outdated Hide outdated railties/lib/rails/generators/actions.rb
@frodsan

View changes

Show outdated Hide outdated railties/lib/rails/generators/actions.rb
@frodsan

View changes

Show outdated Hide outdated railties/lib/rails/generators/base.rb
@frodsan

This comment has been minimized.

Show comment
Hide comment
@frodsan

frodsan Dec 19, 2012

Contributor

👍 Like it.

Contributor

frodsan commented Dec 19, 2012

👍 Like it.

@ndbroadbent

This comment has been minimized.

Show comment
Hide comment
@ndbroadbent

ndbroadbent Dec 19, 2012

Contributor

OK, have updated the commit. Sorry about that, I had started working from the 3-2-stable branch, and forgot to update the coding style :)

Contributor

ndbroadbent commented Dec 19, 2012

OK, have updated the commit. Sorry about that, I had started working from the 3-2-stable branch, and forgot to update the coding style :)

@tilsammans

This comment has been minimized.

Show comment
Hide comment
@tilsammans

tilsammans Dec 19, 2012

Contributor

Very cool.

Contributor

tilsammans commented Dec 19, 2012

Very cool.

@carlosantoniodasilva

View changes

Show outdated Hide outdated railties/lib/rails/generators/actions.rb
@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Dec 19, 2012

Member

It'll also needs some tests, and a changelog entry.

Member

carlosantoniodasilva commented Dec 19, 2012

It'll also needs some tests, and a changelog entry.

@ndbroadbent

This comment has been minimized.

Show comment
Hide comment
@ndbroadbent

ndbroadbent Dec 19, 2012

Contributor

@carlosantoniodasilva, I have added tests and a changelog entry. Thanks

Contributor

ndbroadbent commented Dec 19, 2012

@carlosantoniodasilva, I have added tests and a changelog entry. Thanks

@ndbroadbent

This comment has been minimized.

Show comment
Hide comment
@ndbroadbent

ndbroadbent Dec 19, 2012

Contributor

P.S. Is there any chance this could also be accepted into 3-2-stable? I've prepared a open_generated_files_in_editor_32stable branch if so.

Contributor

ndbroadbent commented Dec 19, 2012

P.S. Is there any chance this could also be accepted into 3-2-stable? I've prepared a open_generated_files_in_editor_32stable branch if so.

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Dec 19, 2012

Member

@ndbroadbent sorry, 3-2 doesn't get any new feature, only bug fixes now. But don't worry, 4.0 beta is coming ;).

I'll take a look later. Thanks.

Member

carlosantoniodasilva commented Dec 19, 2012

@ndbroadbent sorry, 3-2 doesn't get any new feature, only bug fixes now. But don't worry, 4.0 beta is coming ;).

I'll take a look later. Thanks.

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva
Member

carlosantoniodasilva commented Dec 19, 2012

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Dec 19, 2012

Member

This is already out of date.

@ndbroadbent if you want this in 3-2-stable, you can run rails with your own custom patches pretty easily.

Member

steveklabnik commented Dec 19, 2012

This is already out of date.

@ndbroadbent if you want this in 3-2-stable, you can run rails with your own custom patches pretty easily.

@ndbroadbent

This comment has been minimized.

Show comment
Hide comment
@ndbroadbent

ndbroadbent Dec 19, 2012

Contributor

@steveklabnik - rebased, and thanks for the tip!

Contributor

ndbroadbent commented Dec 19, 2012

@steveklabnik - rebased, and thanks for the tip!

@kinopyo

This comment has been minimized.

Show comment
Hide comment
@kinopyo

kinopyo Mar 1, 2013

Contributor

Like it!

Contributor

kinopyo commented Mar 1, 2013

Like it!

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Mar 1, 2013

Member

My idea is not to open all the files, only the main one for each generator.

For example in the model generator we can open only the model file. We don't need to open the fixture file.

Member

rafaelfranca commented Mar 1, 2013

My idea is not to open all the files, only the main one for each generator.

For example in the model generator we can open only the model file. We don't need to open the fixture file.

@arunagw

This comment has been minimized.

Show comment
Hide comment
@arunagw

arunagw May 3, 2013

Member

@ndbroadbent Hey did you got time to work on this again?

Member

arunagw commented May 3, 2013

@ndbroadbent Hey did you got time to work on this again?

@ndbroadbent

This comment has been minimized.

Show comment
Hide comment
@ndbroadbent

ndbroadbent Jul 3, 2013

Contributor

Hey @arunagw, thanks for the reminder! I've rebased and rewritten this so that it only opens the main file for each generator.

Contributor

ndbroadbent commented Jul 3, 2013

Hey @arunagw, thanks for the reminder! I've rebased and rewritten this so that it only opens the main file for each generator.

# open_file_in_editor "app/models/account.rb"
def open_file_in_editor(path)
# Attempt to only open the main file generated by the base generator.
if shell.base.class === self || shell.base.class.invocations.key?(:orm)

This comment has been minimized.

@ndbroadbent

ndbroadbent Jul 3, 2013

Contributor

Not sure about this line, but it does the job. If you run the controller generator, it will only open the controller file in the editor, and ignore the helper file. But if you run the helper generator, it opens the helper file in your editor. The :orm condition is so that the active_record migration and model generators are allowed to open their files.

@ndbroadbent

ndbroadbent Jul 3, 2013

Contributor

Not sure about this line, but it does the job. If you run the controller generator, it will only open the controller file in the editor, and ignore the helper file. But if you run the helper generator, it opens the helper file in your editor. The :orm condition is so that the active_record migration and model generators are allowed to open their files.

@mikegee

This comment has been minimized.

Show comment
Hide comment
@mikegee

mikegee May 21, 2014

Contributor

This is almost a year stale now. Do we want to try again to get it merged, or give up and just close it?

Contributor

mikegee commented May 21, 2014

This is almost a year stale now. Do we want to try again to get it merged, or give up and just close it?

@matthewd

This comment has been minimized.

Show comment
Hide comment
@matthewd

matthewd May 21, 2014

Member

The current implementation seems pretty intrusive... maybe it would feel better if we had a primary_template (or similar name) method that would call template, plus consider any other special handling that might be due to the "main" output of the task (such as, say, opening it in an editor).

And the parameter description needs an update ("all").

Other than that, I'm 👍 to merge this. Trying to tab-complete the filename of a just-created migration is not fun.

Member

matthewd commented May 21, 2014

The current implementation seems pretty intrusive... maybe it would feel better if we had a primary_template (or similar name) method that would call template, plus consider any other special handling that might be due to the "main" output of the task (such as, say, opening it in an editor).

And the parameter description needs an update ("all").

Other than that, I'm 👍 to merge this. Trying to tab-complete the filename of a just-created migration is not fun.

@arthurnn

This comment has been minimized.

Show comment
Hide comment
@arthurnn

arthurnn May 21, 2014

Member

I like the idea here 👍 Lets merge this, and iterate over it.

Member

arthurnn commented May 21, 2014

I like the idea here 👍 Lets merge this, and iterate over it.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jan 2, 2015

Member

@matthewd not sure if I got you suggestion. Are you saying to extract the open in editor code to a new method or are you thinking in a more generic handler for the main file?

Member

rafaelfranca commented Jan 2, 2015

@matthewd not sure if I got you suggestion. Are you saying to extract the open in editor code to a new method or are you thinking in a more generic handler for the main file?

@matthewd

This comment has been minimized.

Show comment
Hide comment
@matthewd

matthewd Jan 3, 2015

Member

@rafaelfranca the latter. Nothing complex... but just some agnostic way of knowing which file is the interesting one. Maybe as simple as

def create_mailer_file
  destination = File.join('app/mailers', class_path, "#{file_name}.rb")
  template "mailer.rb", destination
  primary_file destination
end

[..]
def primary_file(filename)
  # I'm not sure exactly what, but I can see other things wanting to hook in here

  open_file_in_editor(filename) if options["editor"].present?
end

Largely, it seems neater to keep the raw "open in editor" concept out of the individual generators.

I'm not sure how, but the logic in https://github.com/rails/rails/pull/8553/files#r5010513 could hopefully live there too. Maybe?

Member

matthewd commented Jan 3, 2015

@rafaelfranca the latter. Nothing complex... but just some agnostic way of knowing which file is the interesting one. Maybe as simple as

def create_mailer_file
  destination = File.join('app/mailers', class_path, "#{file_name}.rb")
  template "mailer.rb", destination
  primary_file destination
end

[..]
def primary_file(filename)
  # I'm not sure exactly what, but I can see other things wanting to hook in here

  open_file_in_editor(filename) if options["editor"].present?
end

Largely, it seems neater to keep the raw "open in editor" concept out of the individual generators.

I'm not sure how, but the logic in https://github.com/rails/rails/pull/8553/files#r5010513 could hopefully live there too. Maybe?

def open_file_in_editor(path)
# Attempt to only open the main file generated by the base generator.
if shell.base.class === self || shell.base.class.invocations.key?(:orm)
run("#{options["editor"]} \"#{path}\"")

This comment has been minimized.

@repinel

repinel Jun 14, 2015

Member

Have you tested with command-line editor like vim? Although it opens template, the generator is most likely still running and also outputting to the terminal.

@repinel

repinel Jun 14, 2015

Member

Have you tested with command-line editor like vim? Although it opens template, the generator is most likely still running and also outputting to the terminal.

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Oct 30, 2015

Member

Forcing a rebuild.

Member

sgrif commented Oct 30, 2015

Forcing a rebuild.

@sgrif sgrif closed this Oct 30, 2015

@sgrif sgrif reopened this Oct 30, 2015

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Oct 30, 2015

Member

Ok well I can't get travis to run it, but I pulled this branch down locally and it has failing tests. @ndbroadbent are you still interested in working on this?

Member

sgrif commented Oct 30, 2015

Ok well I can't get travis to run it, but I pulled this branch down locally and it has failing tests. @ndbroadbent are you still interested in working on this?

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Dec 8, 2016

Member

Closing due to inactivity.

Member

sgrif commented Dec 8, 2016

Closing due to inactivity.

y-yagi added a commit to y-yagi/rails that referenced this pull request Jul 19, 2018

Added --editor (-e) option to open generated files in the user's editor
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment