adding require to cli breaks rouge #3623
Comments
|
I'll investigate.
Nope, it just does this: But is possible that Rouge itself is doing what you're suggesting. |
|
You can work around the problem by forcing Rouge to load: |
|
Ultimately, this is a bug in Rouge. It's incorrect to require libraries using |
|
Unless a fix is imminent should the next patch release have a shim in place
until there's an upstream fix to avoid breaking extensions? I've seen this
bug with my extensions as well.
I also recall that upstream was not especially willing to make this change
-- something to do with their CI or test cases (slightly bizarre IMHO).
…On Fri, Apr 17, 2020 at 8:04 AM Dan Allen ***@***.***> wrote:
Ultimately, this is a bug in Rouge. It's incorrect to require libraries
using load in place of require. It causes programs that rely on the
behavior of require to malfunction. Please report this upstream. The
workaround in Asciidoctor is to force Rouge to load by passing -r rouge.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3623 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFEXX5NLMAYJQJ7WBORMRTRM5QEPANCNFSM4MKBU3TQ>
.
--
--
Private or confidential message? Public Key available here
<https://pgp.mit.edu/pks/lookup?search=dan.mulholland%40gmail.com&op=index>
:
|
|
Here's a shim I came up with: unless defined? Rouge.version
begin
require 'rouge'
rescue
old_verbose, $VERBOSE = $VERBOSE, nil
original_load = method :load
def load path
require path
end
require 'rouge'
define_method :load, original_load
$VERBOSE = old_verbose
end
endI don't really see any other way around this since what Rouge is doing is fundamentally an incorrect use of cc: @pyrmont |
|
I know why this is happening with extensions specifically. It's caused by the line include Asciidoctor and the fact that Asciidoctor defines a method method named So this is partially Asciidoctor's fault in that it should not be defining a module method named |
|
Here's a simpler solution: unless defined? Rouge.version
define_method :load, &(Kernel.method :load)
require 'rouge'
end |
|
I suggest you open a ticket on Rouge instead of pinging individual Rouge developers. |
|
There was this upstream comment when the issue was previously raised: rouge-ruby/rouge#886 (comment) |
|
That is a separate issue. Using |
|
In the future, @mojavelinux, please refrain from pinging individual developers. If you open an issue on our tracker, we will happily triage it with the rest of the issues. Our project is volunteer-based and we can't prioritize your needs over others on a whim. |
|
The bug appears to be here, where every single method in the Asciidoctor namespace is leaked into the global scope: It should be sufficient to move that line into the class context that comprises the rest of the file. |
I acknowledge that the side effect is being introduced from the Asciidoctor side and I'm taking that up as an action item to mitigate as well.
That statement was only meant to import the classes into the global scope, but the introduction of It's unfortunate that the global ^ In fact, it's very inconsistent that Ruby doesn't produce a warning when warnings are enabled (same is true for both load and require). I wonder why Ruby decided this was an exception. I guess that's just something we need to be aware of. |
@jneen I had an ongoing discussion with Michael about this issue and I was pinging him by request to let him know if this happened to come up again...and it did. It's rather odd you're replying to a ping that wasn't even directed at you. Our project is also volunteer-based and I know very well that needs of individual community members cannot be prioritized on a whim. Nothing I did suggested I was asking for that treatment. So I kindly ask you to refrain from coming in this project and making accusations. Your comment was uncalled for. |
|
A change has been proposed in Rouge to use I've opened a separate issue in Asciidoctor to drop the use of module_function. See #3625. The short term fix for this issue is to avoid using |
|
@scphantm I don't know if you were following the discussion in the Rouge PR but we've updated our calls to |
I have a doc that runs perfectly fine with the latest of everything and rouge. ruby 2.6.0, adoc 2.0.10, rogue 3.18.0,
now everything runs fine until i attempt to add a new extension with the
--require=switch on the cli. When i do that, i get thisthe extension im trying to add is the
tree-block-macro.rbfrom the asciidoc extensions lab. If i run the sample file in the extension alone, it runs fine. but the moment i add the:source-highlighter: rougeto the sample file, it blows up. I'm not a ruby expert, but it looks to me like instead of--require=adding to the load path, its overwriting the load path and other gems are being loaded wrong.The text was updated successfully, but these errors were encountered: