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

Open gem command #804

Merged
merged 14 commits into from Feb 5, 2014

Conversation

Projects
None yet
3 participants
@quozd
Contributor

quozd commented Feb 1, 2014

Opens gem source code in EDITOR. See #789 for details.

Usage:

Open in default editor:

gem open gem_name

Open using vim:

gem open gem_name -e vim

@quozd quozd referenced this pull request Feb 1, 2014

Closed

Open gem source in editor #789

@quozd

This comment has been minimized.

Show comment
Hide comment
@quozd

quozd Feb 1, 2014

Contributor

hm, looks like spawn function is not present in ruby 1.8.7, fork AFAIK is not cross-platform. Any ideas?

Contributor

quozd commented Feb 1, 2014

hm, looks like spawn function is not present in ruby 1.8.7, fork AFAIK is not cross-platform. Any ideas?

@quozd quozd closed this Feb 1, 2014

@quozd quozd reopened this Feb 1, 2014

end
def get_env_editor
ENV['GEM_EDITOR'] ||

This comment has been minimized.

@drbrain

drbrain Feb 3, 2014

Member

Can you include a description of all the environment variables that the open command will use in the description? Only $EDITOR is mentioned.

@drbrain

drbrain Feb 3, 2014

Member

Can you include a description of all the environment variables that the open command will use in the description? Only $EDITOR is mentioned.

@drbrain

View changes

Show outdated Hide outdated lib/rubygems/commands/open_command.rb
def open_editor path
Dir.chdir(path) do
pid = spawn(@editor, path)

This comment has been minimized.

@drbrain

drbrain Feb 3, 2014

Member

spawn is in ruby 1.9 and newer but RubyGems still supports ruby 1.8.7. Can you switch this to use fork + exec instead here?

@drbrain

drbrain Feb 3, 2014

Member

spawn is in ruby 1.9 and newer but RubyGems still supports ruby 1.8.7. Can you switch this to use fork + exec instead here?

@drbrain

View changes

Show outdated Hide outdated lib/rubygems/commands/open_command.rb
pid = spawn(@editor, path)
Process.detach(pid)
end
#unless Gem::Util.silent_system(@editor, path)

This comment has been minimized.

@drbrain

drbrain Feb 3, 2014

Member

Please remove this commented-out code.

@drbrain

drbrain Feb 3, 2014

Member

Please remove this commented-out code.

@drbrain

This comment has been minimized.

Show comment
Hide comment
@drbrain

drbrain Feb 3, 2014

Member

Are there any features from gem-open or gem-edit that are missing here? I don't see any shared code with the existing implementations.

/c @tpope @fnando

Member

drbrain commented Feb 3, 2014

Are there any features from gem-open or gem-edit that are missing here? I don't see any shared code with the existing implementations.

/c @tpope @fnando

@tpope

This comment has been minimized.

Show comment
Hide comment
@tpope

tpope Feb 3, 2014

Contributor

As I read it, this won't work if the editor option or variable contains an argument, for example gvim -f. The best solution I know of is to split on /\s+/.

Other than that, looks great.

Contributor

tpope commented Feb 3, 2014

As I read it, this won't work if the editor option or variable contains an argument, for example gvim -f. The best solution I know of is to split on /\s+/.

Other than that, looks great.

@quozd

This comment has been minimized.

Show comment
Hide comment
@quozd

quozd Feb 4, 2014

Contributor

looks like I have some issues with ruby 1.8, will take a look and fix in the evening. Don't have proper environment right now.

Other remarks were fixed

Contributor

quozd commented Feb 4, 2014

looks like I have some issues with ruby 1.8, will take a look and fix in the evening. Don't have proper environment right now.

Other remarks were fixed

@quozd

This comment has been minimized.

Show comment
Hide comment
@quozd

quozd Feb 4, 2014

Contributor

Guys, I fixed all issues related to my code, but some tests are failing with ruby 1.8.7. I have no idea how my code could influence to this tests. Could somebody take a look and show me where the issue is?
cast @drbrain

Contributor

quozd commented Feb 4, 2014

Guys, I fixed all issues related to my code, but some tests are failing with ruby 1.8.7. I have no idea how my code could influence to this tests. Could somebody take a look and show me where the issue is?
cast @drbrain

@drbrain

This comment has been minimized.

Show comment
Hide comment
@drbrain

drbrain Feb 4, 2014

Member

Those tests are also failing on master so you're good to go!

I'll fix those failing tests and merge your pull request shortly.

Member

drbrain commented Feb 4, 2014

Those tests are also failing on master so you're good to go!

I'll fix those failing tests and merge your pull request shortly.

drbrain added a commit that referenced this pull request Feb 5, 2014

@drbrain drbrain merged commit d92087e into rubygems:master Feb 5, 2014

1 check failed

default The Travis CI build failed
Details

drbrain added a commit that referenced this pull request Feb 5, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment