Skip to content
This repository has been archived by the owner on Mar 3, 2022. It is now read-only.

Fix uncaught LoadError requiring pygments.rb #282

Merged
merged 1 commit into from
Apr 8, 2016

Conversation

davidcelis
Copy link
Contributor

If ghi was installed via Homebrew before pygments.rb was added as a
dependency, the check for pygments being installed never gets run. It
looks like there was intention to rescue that require statement but
because LoadError doesn't inherit from StandardError, the exception
was uncaught and ghi would silently exit. This only seems to happen
currently on issues that include fenced code blocks, but I provided
reproducible evidence in #281. Merging this will fix #281, but I have
two concerns about the fact that pygments.rb was included as a
dependency in the first place:

  1. Homebrew doesn't install Ruby gems for you, so this has to be done
    manually. Attempting to brew install ghi without first having run
    gem install pygments.rb just outputs an error and doesn't finish.
  2. Most Ruby developers install several versions of Ruby using a version
    manager like rvm, rbenv, or chruby. You can gem install pygments.rb
    on one version of Ruby (or sudo gem install pygments.rb on the
    system version of Ruby, which is a whole other problem) but as soon
    as you try to use ghi after switching Rubies, you have to remember
    to install pygments.rb. This isn't great.

I see two solutions here. We can either no longer offer a Homebrew
formula, instead urging users to just install via gem install ghi or
we can remove pygments.rb as a dependency. Personally I think ghi
should try to remain free of dependencies for this reason, but I leave
it in your hands.

If `ghi` was installed via Homebrew before pygments.rb was added as a
dependency, the check for pygments being installed never gets run. It
looks like there was intention to rescue that `require` statement but
because `LoadError` doesn't inherit from `StandardError`, the exception
was uncaught and `ghi` would silently exit. This only seems to happen
currently on issues that include fenced code blocks, but I provided
reproducible evidence in stephencelis#281. Merging this will fix stephencelis#281, but I have
two concerns about the fact that pygments.rb was included as a
dependency in the first place:

1. Homebrew doesn't install Ruby gems for you, so this has to be done
   manually. Attempting to `brew install ghi` without first having run
   `gem install pygments.rb` just outputs an error and doesn't finish.
2. Most Ruby developers install several versions of Ruby using a version
   manager like rvm, rbenv, or chruby. You can `gem install pygments.rb`
   on one version of Ruby (or `sudo gem install pygments.rb` on the
   system version of Ruby, which is a whole other problem) but as soon
   as you try to use `ghi` after switching Rubies, you have to remember
   to install `pygments.rb`. This isn't great.

I see two solutions here. We can either no longer offer a Homebrew
formula, instead urging users to just install via `gem install ghi` or
we can remove pygments.rb as a dependency. Personally I think `ghi`
should try to remain free of dependencies for this reason, but I leave
it in your hands.

Signed-off-by: David Celis <me@davidcel.is>
@stephencelis
Copy link
Owner

Nice catch!

@stephencelis stephencelis merged commit 5abbfb5 into stephencelis:master Apr 8, 2016
@davidcelis davidcelis deleted the fix-parsing branch April 8, 2016 20:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ghi fails to parse some issues and comments, blank output for ghi show
2 participants