search

trulia / hologram

A markdown based documentation system for style guides.
http://trulia.github.io/hologram
Other
2.16k stars 200 forks source link

Incorrect output paths for dependencies/assets on second and subsequent runs of `hologram` #203

Closed tomdavies closed 9 years ago

tomdavies commented 9 years ago

I'm seeing an unexpected behaviour with hologram 1.3.0 when running the shell command more than once (as part of our watch task in local development).

The output path for dependencies is incorrect on the second and subsequent runs, nesting the dependencies inside the dependencies directory created on the first run, instead of replacing it (i.e. I get dependency paths like ./docs/build/build/ when I expect ./docs/build/)

Reduced test case

The hologram example repo exhibits this behaviour, though I'm also seeing it in our dev setup too. I had to rename config.yml to hologram_config.yml to get the example repo to work but that's a different PR :)

  1. Clone the example repo: https://github.com/trulia/hologram-example & bundle install
  2. run hologram in shell
  3. run hologram in shell again

    What I expected to happen

After the second run of the command I expected the structure of the ./docs/ directory to look exactly the same, with any changes to files in ./build/ reflected in ./docs/build/, i.e.

docs
\-build
    |-css
    \-script

What actually happens

After the second run of the command I get a directory structure like this:

docs
|-build
    |-build
    |   |-css
    |   \-script
    |-css
    \-script

i.e. a newly copied instance of build/ inside the original one.

Running hologram again has the same effect of copying ./build/ to ./docs/build/build/)

PS FWIW this is with ruby 2.1.5 on os x

jdcantrell commented 9 years ago

Can you update your gem and try again (from rubygems, or dev if you like) This should be fixed in the 1.3.1 release.

tomdavies commented 9 years ago

You're completely right, sorry could have sworn I was running latest. Many thanks...

jdcantrell commented 9 years ago

Thanks! I'm glad to hear it's fixed.

evanblack commented 9 years ago

I was looking at this today and think it's still an issue in 1.3.1. I saw the change in 1.3.1 on line 164 of doc_builder.rb to change 'rm' to a recursive 'rm_r', but that's only for the copy_dependencies method. Shouldn't this also be done for the copy_assets method?

jdcantrell commented 9 years ago

It's a good point, I'll take a look at that.