search

rrthomas / hpmor

PDF, ePUB and Mobi versions of “Harry Potter and the Methods of Rationality”, from LaTeX source
http://hpmor.com/
295 stars 55 forks source link

Problem with ebook scripts #132

Closed rrthomas closed 1 year ago

rrthomas commented 1 year ago

The ebook scripts contain the idiom:

script_dir=$(cd $(dirname $0) && pwd)
cd $script_dir/../..

There are two problems here:

  1. It's not great for scripts to change directory like this. I would be happier to see script_dir used as a prefix to paths where needed.
  2. cd outputs text to stdout when it matches an entry in CDPATH. I have . in my CDPATH, so most times I use cd, there is output. This breaks the script. It would of course be possible just to unset CDPATH, but is there some reason why the line above can't be replaced with just:
script_dir=$(dirname $0)

? (WFM!)

entorb commented 1 year ago

I used this to to ensure that the current working dir is the hpmor base.

If there is a more elegant way to do it, I am happy to change it.

Alternatively, instead of cd to that dir, I could perform a check for the presence of "chapters/" dir in current dir and exit if not found.

rrthomas commented 1 year ago

I think the simplest change here is the "more elegant way" I suggest in number 2 above; does that work for you? It should be correct, but I wonder why a more complicated command was used before…

entorb commented 1 year ago

Thanks for pointing this out, fix is proposed in https://github.com/rrthomas/hpmor/pull/136

rrthomas commented 1 year ago

Thanks, this fixes things for me, closing.