Skip to content
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

Already initialized constant Rouge::Lexers:Shell::KEYWORDS #296

Closed
stanhu opened this issue Jul 30, 2015 · 19 comments
Closed

Already initialized constant Rouge::Lexers:Shell::KEYWORDS #296

stanhu opened this issue Jul 30, 2015 · 19 comments

Comments

@stanhu
Copy link
Contributor

stanhu commented Jul 30, 2015

Now that GitLab is using Rouge, it appears that Rouge is loading files automatically, causing Rails to warn with errors:

/Users/stanhu/.rbenv/versions/2.1.6/lib/ruby/gems/2.1.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:20: warning: already initialized constant Rouge::Lexers::Shell::KEYWORDS
/Users/stanhu/.rbenv/versions/2.1.6/lib/ruby/gems/2.1.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:20: warning: previous definition of KEYWORDS was here
/Users/stanhu/.rbenv/versions/2.1.6/lib/ruby/gems/2.1.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:25: warning: already initialized constant Rouge::Lexers::Shell::BUILTINS
/Users/stanhu/.rbenv/versions/2.1.6/lib/ruby/gems/2.1.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:25: warning: previous definition of BUILTINS was here

These loads happen twice:

  1. https://github.com/jneen/rouge/blob/master/lib/rouge/lexers/powershell.rb#L5
  2. https://github.com/jneen/rouge/blob/master/lib/rouge.rb#L43

Removing these lines seems to make the warnings go away. Is it possible to remove these automatic loads?

@jacobvosmaer
Copy link

Sounds like a use case for Ruby's require instead of load.

@rumpelsepp
Copy link
Contributor

Among others I had replaced these load calls with require relative. I forgot to file a pull request for this.
https://github.com/rumpelsepp/rugments/commit/e6bdd4eeed47670710d48f53c4b7759a35f1f348

@stanhu
Copy link
Contributor Author

stanhu commented Jul 31, 2015

@rumpelsepp Thanks. Took your work and made it work for the specs and rackup application in #298.

@rumpelsepp
Copy link
Contributor

👍

@tejasbubane
Copy link
Contributor

This happens in irb as well.
gem version 1.9.1.

/lib/rouge/lexers/shell.rb:20: warning: already initialized constant Rouge::Lexers::Shell::KEYWORDS
/lib/rouge/lexers/shell.rb:20: warning: previous definition of KEYWORDS was here
/lib/rouge/lexers/shell.rb:25: warning: already initialized constant Rouge::Lexers::Shell::BUILTINS
/lib/rouge/lexers/shell.rb:25: warning: previous definition of BUILTINS was here

@envygeeks
Copy link

/cc @jneen

@nhoizey
Copy link

nhoizey commented Aug 4, 2015

I also have the issue with Jekyll and Kramdown…

@JoshCheek
Copy link
Contributor

Why did they have the loads in the first place? Also, this is introduced in c8bf4a8

This is an alternative that maintains the load behaviour (again, IDK why it's there, a guess would be for reloading in rack when working tweaking the visual themes, but are better ways to achieve that, it does seem like the right solution would be to use require, except for in the case of rack, where we would want to add autoloading)

diff --git a/lib/rouge/lexers/shell.rb b/lib/rouge/lexers/shell.rb
index 8b7134e..2a7acea 100644
--- a/lib/rouge/lexers/shell.rb
+++ b/lib/rouge/lexers/shell.rb
@@ -17,12 +17,14 @@ module Rouge
         text.shebang?(/(ba|z|k)?sh/) ? 1 : 0
       end

-      KEYWORDS = %w(
+      KEYWORDS = "" unless defined? KEYWORDS
+      KEYWORDS.replace %w(
         if fi else while do done for then return function
         select continue until esac elif in
       ).join('|')

-      BUILTINS = %w(
+      BUILTINS = "" unless defined? BUILTINS
+      BUILTINS.replace %w(
         alias bg bind break builtin caller cd command compgen
         complete declare dirs disown echo enable eval exec exit
         export false fc fg getopts hash help history jobs kill let

JoshCheek added a commit to JoshCheek/what-we-ve-got-here-is-an-error-to-communicate that referenced this issue Aug 17, 2015
It winds up loading the same file twice,
causing constants to be redefined,
which causes warnings to print every time it is run.
See rouge-ruby/rouge#296 for more details.
@gpakosz
Copy link
Contributor

gpakosz commented Aug 20, 2015

Could this be fixed with a 1.9.2 maintenance release please?

@JoshCheek
Copy link
Contributor

Aye, I've dealt with it by disallowing 1.9.1

@envygeeks
Copy link

Thanks @JoshCheek I totally did not know we could do what you did in a Gemspec.

@sc0ttwad3
Copy link

Still getting the issue... during Jekyll generations currently... (on Windows boxes as well -- below). Has @JoshCheek found a workaround? Steps involved?

C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:20: warning: already initialized constant Rouge::Lexers::Shell::KEYWORDS
C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:20: warning: previous definition of KEYWORDS was here
C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:25: warning: already initialized constant Rouge::Lexers::Shell::BUILTINS
C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/rouge-1.9.1/lib/rouge/lexers/shell.rb:25: warning: previous definition of BUILTINS was here

@stanhu
Copy link
Contributor Author

stanhu commented Aug 27, 2015

I suspect the loads were used to make the visual test rackup work. If you make a change in the code, the loads will pull in the new code automatically. #298 emulates this behavior by manipulating the LOADED_FEATURES value.

@sc0ttwad3
Copy link

Great Ill give it a try, thx.

On Thu, Aug 27, 2015, 6:07 PM Stan Hu notifications@github.com wrote:

I suspect the loads were used to make the visual test rackup work. If you
make a change in the code, the loads will pull in the new code
automatically. #298 #298 emulates
this behavior by manipulating the LOADED_FEATURES value.


Reply to this email directly or view it on GitHub
#296 (comment).

@wolf99
Copy link

wolf99 commented Aug 31, 2015

👍

@divoxx
Copy link

divoxx commented Sep 4, 2015

Why is load used everywhere instead of require ?

@gpakosz
Copy link
Contributor

gpakosz commented Jun 6, 2016

Which commit closes this issue please?

@jneen
Copy link
Member

jneen commented Jun 6, 2016

65c69f0. It's fixed as of 1.10.0.

@gpakosz
Copy link
Contributor

gpakosz commented Jun 6, 2016

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests