asciidoctor / asciidoctor

:gem: A fast, open source text processor and publishing toolchain, written in Ruby, for converting AsciiDoc content to HTML 5, DocBook 5, and other formats.
https://asciidoctor.org
Other
4.79k stars 788 forks source link

Incorrect html rendering with passthrough #734

Open davebaldwin opened 10 years ago

davebaldwin commented 10 years ago

I have been using coderay via an extension and have been getting incorrect rendering. If I include the same source code via [source,c] mechanism it renders fine, but I have cut down the problem and can reproduce it using pass through block. Rendering the code as given will show the problem which starts on the line beginning with indexMask = ... . The two opening parenthesis are missing and all the other lines are concatenated.

If one of the two closing parenthesis on the last but 3 line is removed then rendering is corrected.

I don't think there is a problem with my extension as I can reproduce the problem no using it but here is a cut down verion iin case it helps through some light on the matter.

class BSourceCode < Asciidoctor::Extensions::BlockMacroProcessor
    def process (parent, target, attributes)
        src_dir = @document.attributes.fetch('src_dir', '')
        src_file = File.join(src_dir, target)
        css = @document.attributes.fetch('coderay-css','class').to_sym
        source = CodeRay::Duo.new(:c, :html, :css => css).highlight(File.read(src_file))

        content = %Q{
            <div class="listingblock">
                <div class="content">
                    <pre class="CodeRay"><code class="c language-c"> 
                    #{source}
                </div>
            </div>
        }

        Asciidoctor::Block.new parent, :pass, :content_model => :raw, :source => content    
    end
end

Dave.

:doctype: book
:source-highlighter: coderay
:linkcss:
 

    switch (indexBuffer.IndexFormat)
    {
        case eI_BYTE:   sizeOfIndex = 1; break;
        case eI_WORD:   sizeOfIndex = 2; break;
        case eI_DWORD:  sizeOfIndex = 4; break;
        default:        return 0;        break;
    }  

    indexMask = 0xffffffff >> ((4 - sizeOfIndex) * 8);
    byteAddress = indexBuffer.BufferStartingAddress + offset;

    memAddr = byteAddress >> 6;

}

            case eUNSIGNED_BYTE:                // fall through
            case eBYTE:                         
                bytes = 1;
                break;

            case eUNSIGNED_SHORT:               // fall through
            case eSHORT:                        // fall through
            case eHALF_FLOAT:
                bytes = 2;   
                break;

            8)) & 0xff;
mojavelinux commented 10 years ago

Dave,

The problem you are running into is that the C code is getting caught up in an index term substitution. This happens because, by default, pass blocks are subject to attributes and macros substitutions. Obviously, this isn't what you want.

You can easily disable substitutions by setting the subs attribute to none when you initialize the block.

Asciidoctor::Block.new parent, :pass, :content_model => :raw,
    :source => content, :attributes => {'subs' => 'none'}

The combination of a :pass block with subs of none achieves strictly raw output.

I'd like to propose a simpler approach to your use case. As I understand, you are creating a block macro for including source from an external file. All your extension needs to do is prepare a listing block, and the source highlighting will happen automatically.

Here's how that extension would look:

# Processes a block macro for inserting syntax highlighted
# source code read from an external file.
#
# Example:
#
# source::sample.c[]
#
class BSourceCode < Asciidoctor::Extensions::BlockMacroProcessor
    def process (parent, target, attributes)
        src_dir = @document.attr 'src_dir', '.'   # <1>
        src_file = File.join src_dir, target
        src_lang = (File.extname src_file)[1..-1] # <2>
        src_code = File.read src_file
        # must modify attribute reference to set attributes
        attributes['style'] = 'source'            # <3>
        attributes['language'] ||= src_lang       # <4>
        Asciidoctor::Block.new parent, :listing, :content_model => :verbatim,
            :source => src_code, :attributes => attributes
    end
end
  1. Reads the document attribute src_dir, falling back to the current directory
  2. Uses the source file extension to determine the language
  3. Upgrades the listing block to a source listing block
  4. Sets the language on the block unless it was set in the macro itself

This would definitely make a nice example for the documentation. I could see next step being the ability to filter the source by lines and tagged regions like the include macro.

Note that there are some utilities internally to resolve path locations, which would be useful in this macro. I'll be sure to document those as part of the example.

mojavelinux commented 10 years ago

@graphitefriction Let's make this part of the extension chapter of the user manual.

mojavelinux commented 10 years ago

I'm also going to open an issue to disable macro subsitutions for pass through blocks by default. I just don't think it makes any sense at all...and if you really need it you can always add it back in.

graphitefriction commented 10 years ago

@mojavelinux Can this example be put in the documentation now, or should I wait until you:

"Note that there are some utilities internally to resolve path locations, which would be useful in this macro. I'll be sure to document those as part of the example."

davebaldwin commented 10 years ago

Thanks for the simple fix. I wasn't expecting a pass through block to be partially processed and think the change of default behaviour is a good thing.

The extension code I posted omitted the useful stuff (to me) for clarity and to show it wasn't my extras that were causing the problem. The missing parts were to hyperlink function calls to the function definition and to prepare the data to allow them to be spliced into the TOC as well so using the builtin listing block doesn't work for me.

I started a thread on the discussion list about this and presented a solution (I have since found the solution is incomplete in that it misses out function definitions returning a non standard type, but haven't got around to fixing this yet).

Dave.

mojavelinux commented 10 years ago

Thanks for the simple fix. I wasn't expecting a pass through block to be partially processed and think the change of default behaviour is a good thing.

Given that it surprised me too, and I can't see when I would want those defaults, I'll go file it right away.

The missing parts were to hyperlink function calls to the function definition and to prepare the data to allow them to be spliced into the TOC as well so using the builtin listing block doesn't work for me.

Ah, that's right! We'll, now you're armed with the sample code to use in either case :) Thanks for being an early adopter of the extension API. Keep the feedback coming!