cwjohan / markdown-to-html

Command-line utility to convert Github Flavored Markdown to HTML.
MIT License
236 stars 86 forks source link

Fix Interpreter: execute via shell #6

Closed chalos closed 7 years ago

chalos commented 8 years ago

Hi, @cwjohan This is fixes #2

why use shell or why not use she-bang solution?

node installed location is differ from others, so sha-bang would also differ from others.

instead of changing

#!node

to

#!/bin/node 
or
#!/use/local/node 
or
#!/any/possible/location/of/node 

problem still there.

safer way to execute command

$ node markdown file.md [<option>]

So write the command in shell script, let the shell script execute the command

replace markdown with shell script rename markdown to markdown_main

cwjohan commented 8 years ago

Thank you, Ryan, for the suggestion. I think this just moves the problem from one script to another, though. Correct me if I'm wrong about that.

Everyone, is it unreasonable to ask that node be on the user's path so that

!node will work? This solution requires no script changes.

The remaining problem is that some users experience end of line or script encoding issues, which I presume are easily fixed, though I'm not sure what the most portable permanent solution is (i.e., one that works for everyone without the user having to make any source code changes). Suggestions are welcome if they keep that goal in mind. We can't have a solution that works only for Linux users or only for Windows users.

On Wed, Oct 21, 2015 at 11:06 PM, Chalos Yance Lawrence < notifications@github.com> wrote:

fixes #2 https://github.com/cwjohan/markdown-to-html/issues/2 why use shell or why not use she-bang solution?

node installed location is differ from others, so sha-bang would also differ from others.

instead of changing

!node

to

!/bin/node

or#!/use/local/node or#!/any/possible/location/of/node

problem still there. safer way to execute command

$ node markdown file.md [

So write the command in shell script, let the shell script execute the command

replace markdown with shell script https://github.com/chalos/markdown-to-html/blob/facc5d0fb6d041d902bff122f16fab84349214d0/bin/markdown rename markdown to markdown_main

https://github.com/chalos/markdown-to-html/blob/facc5d0fb6d041d902bff122f16fab84349214d0/bin/markdown_main

You can view, comment on, or merge this pull request online at:

https://github.com/cwjohan/markdown-to-html/pull/6 Commit Summary

  • Fix Interpreter

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/cwjohan/markdown-to-html/pull/6.

ashsearle commented 8 years ago

TL;DR: Common / best / essential practice for portability is to use absolute paths in the she-bang line and avoid putting a carriage-return at end-of-line (i.e. use unix format, not DOS.)

Everyone, is it unreasonable to ask that node be on the user's path so that

!node will work? This solution requires no script changes.

I have node on the path, but you'll see (on OSX at least) a bare #!node shebang doesn't work:

$ which -a node
/usr/local/bin/node
$ ls -l relative.sh using-env.sh 
-rwxr-xr-x  1 ash  staff  27  6 Dec 14:20 relative.sh
-rwxr-xr-x  1 ash  staff  40  6 Dec 14:22 using-env.sh
$ cat relative.sh && ./relative.sh 
#!node
console.log("OK");

-bash: ./relative.sh: node: bad interpreter: No such file or directory

$ cat using-env.sh && ./using-env.sh 
#!/usr/bin/env node
console.log("OK");

OK

If I look at the other npm packages I've got installed, I see EVERY OTHER node script is installed with #!/usr/bin/env node - that includes major packages like webpack, bower, grunt, babel, phantomjs, ...

$ head -1 $(npm config get prefix)/bin/* | grep '#!' | sort | uniq -c
  23 #!/usr/bin/env node
   4 #!node

Those 4 scripts using #!node are all yours.

Second issue, unix vs DOS file format - your scripts (in DOS format) aren't recognised as executables - checked using file:

$ cd $(npm config get prefix)/bin
$ file *
babel:                  a node script text executable
babel-node:             a node script text executable
bower:                  a node script text executable
...
github-markdown:        ASCII C++ program text, with CRLF line terminators
github-markdownb:       ASCII C++ program text, with CRLF line terminators
markdown:               ASCII C++ program text, with CRLF line terminators
markdownb:              ASCII C++ program text, with CRLF line terminators
...
webpack:                a node script text executable
webpack-dev-server:     a node script text executable
yo:                     a node script text executable

If I JUST change the shebang line, I end up with:

$ markdownb 
env: node\r: No such file or directory

After changing the file-format to unix (and therefore removing the carriage return after node) the scripts work fine, but no-one should have to do this manually.

vol7ron commented 7 years ago
  1. I have never seen a pathless executable (#!node), but I am not a shell master. When I first saw that in this project, I was curious if that was a newer hack that I could use.
  2. I also agree that node markdown <file> would be preferential
  3. I thought the script might generate the HTML file by default, instead of only outputting to STDOUT; that is, I thought this:
node markdown /some/file.md

would be the same as this:

node markdown /some/file.md > /some/file.html

I suppose I could see how no STDOUT redirection is desired, perhaps my expectation was uncommon. I was also hoping to full HTML scaffolding, with embedded default CSS style.

l.