lggruspe / slipbox

A static site generator for Zettelkasten notes
https://lggruspe.github.io/slipbox
MIT License
57 stars 5 forks source link

add specific error message for missiing graphviz dependency #18

Closed epogrebnyak closed 1 year ago

epogrebnyak commented 2 years ago

Thanks a lot for the beautiful generator for the notes. I think it is really a missing link in the ecosystem - too many editors, to few open free presenatations of notes, in my opinion.

I spotted a problem which prevents me from running slipbox: the dependency check:

Q:\slipbox
λ slipbox
pandoc not found

Q:\slipbox
λ python -c"from shutil import which; print(which('pandoc'))"
C:\Users\epogr\Anaconda3\Scripts\pandoc.EXE

pandoc is available on the system, but fails the check by slipbox, wonder why that happens. Maybe aoid dependecy check at all and allow to fail when called and not found?

epogrebnyak commented 2 years ago

Aha, I am missing dot, but this is not exactly the error message:

Q:\slipbox
λ python -c"from shutil import which; print(which('dot'))"
None
epogrebnyak commented 2 years ago

So it appears https://github.com/lggruspe/slipbox/blob/f6b1871b01cd5ba78f23b6577ad537b32623ef8a/slipbox/__main__.py#L30-L31 checks for both pandoc and dot, but the error message will be pandoc not found even if pandoc is present and dot is missing.

I can split the check into two functions and add extra error message if you'd advise me to.

My initial problem is solved by installing graphviz, and I'm looking forward to experimenting with slipbox, very excited about it.

epogrebnyak commented 2 years ago

https://github.com/lggruspe/slipbox/blob/f6b1871b01cd5ba78f23b6577ad537b32623ef8a/slipbox/dependencies.py#L8-L14

epogrebnyak commented 2 years ago

Probably even a more simple fix for a PR is adding more verbose error message, eg "Either pandoc or graphviz not found, aborting..."

or

to do has_pandoc(app) and has_dot(app) in dependency.py (which I think simplifies the check a lot and allows to separate the error handling):

def has_pandoc(app):
   return which(app.config.pandoc)

def has_dot(app):
   return which(app.config.dot)
lggruspe commented 2 years ago

I would go with the second option, so the error messages can be more specific.

lggruspe commented 1 year ago

fixed in c15afe0a751e8df78a98d2432ee583a0c801c7a1

epogrebnyak commented 1 year ago

Thank you for following up on this @lggruspe !