search

amyreese / markdown-pp

Preprocessor for Markdown files to generate a table of contents and other documentation needs
MIT License
309 stars 68 forks source link

To what location should the filepath be relative in an !INCLUDE instruction? #10

Open eriksank opened 10 years ago

eriksank commented 10 years ago

I have tried to make it relative to the current working directory, but markdown-pp does not seem to find the file. I tried to make it relative to the location of the file being processed, but that does not work either. How does it work?

By the way, markdown-pp does not give any error message when it cannot find a an !INCLUDE file. It just produces an empty file in my case ...

amyreese commented 10 years ago

the path should be relative to the location of the file that contains the !INCLUDE line. It's quite possible that the logic for searching is not correct though. The module responsible for this is https://github.com/jreese/markdown-pp/blob/master/MarkdownPP/Modules/Include.py - any improvements to the code you can offer would be appreciated.

eriksank commented 10 years ago

It seemed that the CWD mattered at some point, actually. I am not sure, however, because I have just moved on and developed a workaround to the issue.

I have solved it by making the CWD the same as the location of the file that contains the !INCLUDE directives. As a result, whatever it is relative to, it will still work.

That means, however, that I had to manually concatenate all the files first in order to produce the final document which contains the embedded !INCLUDE directives. It was virtually impossible to get markdown-pp to do this, because of the confusing relative path issue (relative to what?).

I am rather a NodeJS than a Python afficionado. So, I do not have sufficient and immediately available Python knowledge about the details of its APIs.

But then again with the workaround, it all works fine. You can see the result here: https://github.com/eriksank/rpc-websocket

The documentation sections that I concatenated to get the final README.md file, are here: https://github.com/eriksank/rpc-websocket/tree/master/doc/readme

Creating an index.md file with this list of files in the doc/readme folder (which need to be concatenated), under the form of successive !INCLUDE directives would actually take as much time as the current approach. So, I am ok with the current approach.

It would just be a good idea to test if markdown-pp really does use the containing file's path as the basis to compute the relative path of its include directives; and, if it really honours "../" and "./" indications. What you need, I guess, is a bunch of elaborate test cases (unit tests).

For my own publications, I spend much more time on examples, the unit tests, and the documentation than the code itself ;-)

leotrs commented 8 years ago

@jreese I have come across the same issue while trying to write a test for the --recursive flag.

Right now, paths included with !INCLUDE default to being relative to where the command was executed. It seems there was an attempt to use file-relative paths on Include.py#56 but the pwd variable is never used.

I agree that paths should be relative to the file that uses the !INCLUDE directive. One way to solve it is make MarkdownPP receive paths at construction time instead of file-like objects. The only drawback would be that every test would have to depend on a physical file, instead of passing around StringIO objects like I was doing. No biggie.

I can prepare a PR if this is the way we want to go.