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

Shell error in generated docs #43

Closed
oubiwann opened this issue Jan 18, 2016 · 15 comments
Closed

Shell error in generated docs #43

oubiwann opened this issue Jan 18, 2016 · 15 comments
Labels
Milestone

Comments

@oubiwann
Copy link
Contributor

So, I spoke too soon about the docs I had generated for the lgeo project :-(

Turns out all the docstrings are rendered with this error instead of the actual docstrings:

/bin/sh: 1: Syntax error: redirection unexpected 

Here's a screenshot:

lodox-screnshot

Note that I'm running on Linux, not Mac OS X ... (there are differences in how they handle redirection).

@yurrriq yurrriq added the bug label Jan 18, 2016
@yurrriq
Copy link
Member

yurrriq commented Jan 18, 2016

Yikes. The pandoc usage is pretty hacky atm. In an ideal world, I'd write a Haskell port and interface with it directly. The offending code is here.

@oubiwann and anyone else reading this, do you have experience writing a Haskell port? Can it be done? Or, is there a better, more platform-independent way to "shell out" to pandoc?

@oubiwann
Copy link
Contributor Author

Hrm, I think (os:cmd) should be just fine. Unfortunately, to debug this we'll have to break up the pretty thrushing :-( 'cause we'll need to see what command is getting generated in Linux.

At the risk of throwing more engineering at the problem, it may be worth setting up lodox to use logjam ... this would allow for easier debugging be interested parties.

If you like that idea, we can open a feature ticket, and I'll get that running so that this issue (and future ones) may be more easily debugged ... it will mean adding an lfe.config file, though (since that's what logjam uses ...)

@yurrriq
Copy link
Member

yurrriq commented Jan 18, 2016

logjam could be cool.

@yurrriq
Copy link
Member

yurrriq commented Jan 18, 2016

Re: breaking up thrushing. doto (or the ! arrows in pynchon) to the rescue!

Something like the following should get the job done. (At work now, so I can't fully dive in...)

(->> `[,pandoc ,(escape markdown)]
                 (doto (io_lib:format "~s -f markdown_github -t html <<< \"~s\"")
                   (->> (list) (io:fwrite "** PANDOC CMD: ~p~n")))
                 (lists:flatten)
                 (os:cmd))

@oubiwann
Copy link
Contributor Author

Oooh, that sounds interesting. I'd like to see that in action!

@oubiwann
Copy link
Contributor Author

Okay, so two problems, one minor.

  1. The following :
(io_lib:format "~s -f markdown_github -t html <<< \"~s\"")

needs to be changed to this:

(io_lib:format "~s -f markdown_github -t html <<< '~s'")

That will prevent Bash from barfind on parens that are in the here-string.

  1. This one's trickier. It seems that sh is what is used on Linux when calling #'os:cmd/1 and sh doesn't understand here-strings on Linux. I got around this by changing the command to "bash -c \"~s\"" (,cmd))". Worked like a charm. However, when I tried to build the command in pieces using #io_lib:format/2, #'os:cmd/1 reverted to usingsh(or something else happened) and it gave the redirection error again. The only way I was able to get this to work was by assembling thecmdstring with++``.

Some examples will illustrate the problem.

Current, broken (on Linux) behaviour:

> (os:cmd "/usr/bin/pandoc -f markdown_github -t html <<< '\\`\\`\\`commonlisp\n(_SRS)\n\\`\\`\\`'")
"/bin/sh: 1: Syntax error: redirection unexpected\n"

Getting it to work with bash -c:

> (os:cmd "bash -c \"/usr/bin/pandoc -f markdown_github -t html <<< '\\`\\`\\`commonlisp\n(_SRS)\n\\`\\`\\`'\"")
"<pre class=\"sourceCode commonlisp\">
<code class=\"sourceCode commonlisp\">(_SRS)</code>
</pre>\n"

Parameterizing the bash -c solution:

> (set cmd "/usr/bin/pandoc -f markdown_github -t html <<< '\\`\\`\\`commonlisp\n(_SRS)\n\\`\\`\\`'")
"/usr/bin/pandoc -f markdown_github -t html <<< '\\`\\`\\`commonlisp\n(_SRS)\n\\`\\`\\`'"
> (os:cmd (lists:flatten (io_lib:format "bash -c ~s" `(,cmd))))
"/bin/sh: 1: Syntax error: redirection unexpected\n"
> (os:cmd (++ "bash -c \"" cmd "\""))              
"<pre class=\"sourceCode commonlisp\">
<code class=\"sourceCode commonlisp\">(_SRS)</code>
</pre>\n"

I'm not submitting a patch for this due to the fact that there are all sorts of ways one might solve this, but the most obvious that works (appending strings) breaks your style (e.g., you won't be able to use the ->> thrushing macro without adding a wrapper functions that takes the args in the appropriate positions).

The approach that I might take would be to define #'bash-cmd/1 and it would append the strings and call `#'os:cmd/``, thus letting you thrush away. This will likely be pretty fragile (though I can't think of a quick, non-fragile work-around), and you may have other ideas ...

oubiwann added a commit to lfeutre/lgeo that referenced this issue Jan 19, 2016
This required hacking the lodox source to support Linux. I used the approach
outlined in this bug report:
 * quasiquoting/lodox#43
@yurrriq
Copy link
Member

yurrriq commented Jan 19, 2016

Thanks for looking into it further. Seems like a tricky, platform specific issue.. bash -c would break on Windows, right?

It seems like writing to temp files would work but that's kind of a last resort. I may explore writing a Haskell port since that sounds fun anyway.

Maybe a cone on host type with platform-specific solutions will suffice for now.

@yurrriq
Copy link
Member

yurrriq commented Jan 19, 2016

👍 for bash-cmd for now though.

@yurrriq yurrriq added this to the 1.0.0 milestone Jan 19, 2016
@yurrriq
Copy link
Member

yurrriq commented Jan 21, 2016

(io_lib:format "~s -f markdown_github -t html <<< '~s'")

breaks code blocks and more, for me.

@yurrriq
Copy link
Member

yurrriq commented Jan 21, 2016

TIL here strings write to a temporary file anyway, so I'm doing it manually with some promising initial success.

yurrriq added a commit that referenced this issue Jan 21, 2016
@yurrriq
Copy link
Member

yurrriq commented Jan 21, 2016

@oubiwann, could you pull develop and try it? In case I push more code, the commit I mean for you to try is 7fee1e4.

@yurrriq
Copy link
Member

yurrriq commented Jan 21, 2016

^ includes some config overhaul see rebar.config for an example.

I'm not married to apps at the top level, but it works nicely when a project has multiple apps.

@yurrriq
Copy link
Member

yurrriq commented Jan 21, 2016

Pasted example for convenience:

{lodox,
 [{apps,
   [{lodox,
     [{'source-uri',
       "https://github.com/quasiquoting/lodox/blob/{version}/{filepath}#L{line}"},
      {'output-path', "relative/path/to/output"}]}]}]}.

N.B. output-path is taken literally, for now, so {version} won't do anything.

@yurrriq
Copy link
Member

yurrriq commented Feb 18, 2016

{version} should work in develop.

@yurrriq
Copy link
Member

yurrriq commented Feb 18, 2016

@oubiwann, does 0.12.5 work for you?

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

No branches or pull requests

2 participants