search

holoviz-dev / nbsite

Build a tested, sphinx-based website from notebooks
https://nbsite.pyviz.org
BSD 3-Clause "New" or "Revised" License
28 stars 13 forks source link

"Page Source" link is broken on site built on nbsite #166

Open dheepakg opened 4 years ago

dheepakg commented 4 years ago

This is with reference with an issue in pyviz.org.

Thanks to @ceball, its been noticed there is a part of code that's deleting folder - '_sources' during the build.

Also, the code block between lines 43-52 in same file is not doing much.

However, the removal of the code blocks, and corresponding effect on the application is not tested in local/dev build. This is to be tested.

jbednar commented 4 years ago

What's the request here?

ceball commented 4 years ago

I think @dheepakg is just following up on the suggestion in https://github.com/pyviz/pyviz.org/issues/57 to track that issue here:

Screenshot from 2020-03-09 16-48-18

dheepakg commented 4 years ago

Sorry for the confusion caused.

'Page Source' of the site generated from nbsite is broken, this needs to be fixed.

dheepakg commented 4 years ago

The original issue is the URL in the footer of pyviz.org is broken. Refer - https://github.com/pyviz/pyviz.org/issues/57 The footer url is pointing to https://pyviz.org/ _sources/index.rst.txt

During nbsite build, we are removing '_sources' directory. Refer - https://travis-ci.org/pyviz/pyviz.org/jobs/657893376#L809

The deletion happens due to the block - https://github.com/pyviz-dev/nbsite/blob/abb56c2a5b1348c97c2b1f2072632dc8a74eba00/scripts/nbsite_cleandisthtml.py#L32

I've made the changes to retain _sources folder - https://github.com/dheepakg/nbsite/blob/master/scripts/nbsite_cleandisthtml.py#L32

However, this code change resulted in failure of one test case - _test_build_deletes_by_default_. Please refer - https://travis-ci.com/github/dheepakg/nbsite/builds/152841644#L641

In this test case, we are expecting 10 items inside "builddocs". But we end up having 11 items inside the folder (count incremented due to _sources).

My question is, should we stick to 10 items inside the folder or the test case should be modified and proceeded with the changes?

Thanks for helping!

dheepakg commented 4 years ago

@jbednar @ceball Can you please check this

ceball commented 4 years ago

If nbsite's going to stop removing _sources now, it's fine to update the test to reflect that.

I think it's no problem in general to leave _sources, and that should have been the default originally. I don't know why it was not, so I feel slight unease we might cause a problem (e.g. maybe some project has huge files going in there or something?). But hopefully someone will speak up if we do cause a problem! Or even better, before then (e.g. maybe @philippjfr or @jsignell, as other frequent victims of nbsite...).

jsignell commented 4 years ago

I think there should probably be a new flag that allows the user to control whether the source is deleted. That way the default could stay the same and downstream libraries can choose whether or not to retain the source.