Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix external CSS stylesheet links for html files within doc sub-directories #39

Merged
merged 6 commits into from
Jun 17, 2021

Conversation

Anirban166
Copy link
Collaborator

The links to the estilo1 CSS file mentioned in the html files within the folders inside doc/ (just saw this while browsing through them) were also linked to the rforge hosted file (http://directlabels.r-forge.r-project.org/estilo1.css), which is not accessible outside, so I changed them to link back to the CSS file in docs/ (I could have also used the one in /root, should I just keep one copy of the css file?) and did the same in templates/head.html for automatic updation when using dldoc

(For example, you can check https://tdhock.github.io/directlabels/docs/contourplot/posfuns/bottom.pieces.html and http://directlabels.r-forge.r-project.org/docs/contourplot/posfuns/bottom.pieces.html to see the difference before merging this)

@tdhock If you find anything falling behind or lacking in https://tdhock.github.io/directlabels/ with respect to the rforge hosted website (http://directlabels.r-forge.r-project.org/), then let me know!

…hin docs/ instead of the rforge hosted file; head template is edited for dldoc runs) for doc sub-directories
@tdhock
Copy link
Owner

tdhock commented Jun 13, 2021

no need for two copies css file. did you find the button / arrow to mark pull request as a draft? https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request

@Anirban166
Copy link
Collaborator Author

no need for two copies css file. did you find the button / arrow to mark pull request as a draft? https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request

Yup thought so, I made the changes and yes, I did find the button, I'll use it for PRs which I'm working on and which are not ready to be merged right away!

@Anirban166
Copy link
Collaborator Author

Anirban166 commented Jun 14, 2021

Some more changes to the templates could include:

  • Replacing install.packages(c("directlabels","ggplot2","ElemStatLearn","mlmRev")) with install.packages(c("ggplot2", "directlabels", "reshape2", "mlmRev", "lars", "latticeExtra")) (excluding inlinedocs, i.e. considering the user would directly want to run dldoc without re-building the skeleton, and ElemStatLearn is no longer required now so removed that) in templates/index.html
  • Replacing http://inlinedocs.r-forge.r-project.org with https://github.com/tdhock/inlinedocs, since you mentioned inlinedocs from rforge is deprecated (although the landing webpage on rforge is helpful, maybe you should host that via GitHub pages and a link to that would be better?) and maybe replacing the link to your contact in templates/foot.html (apart from the link to the directlabels page)

@tdhock what do you think? I can create another PR for these if deemed suitable

@tdhock
Copy link
Owner

tdhock commented Jun 14, 2021

yes please update the templates

@Anirban166
Copy link
Collaborator Author

yes please update the templates

okay, which link of yours should I put for the contact?

@tdhock
Copy link
Owner

tdhock commented Jun 14, 2021

http://tdhock.github.io/

@Anirban166
Copy link
Collaborator Author

@tdhock are the changes okay? anything else that you would like me to change here on this branch or under this PR? (would need to get this merged before #38 - since that will bring changes into gh-pages)

Comment on lines 75 to 76
code version 2014.1.27
(svn revision 675)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be changed eventually, is now a good time?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I'll do it now, thanks for pointing that out!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to include the git version info? The current code (lines 75 to 77) provides the following information:
image

@tdhock
Copy link
Owner

tdhock commented Jun 16, 2021

it is difficult/impossible for me to review this PR because there are so many changes (103 files changed).
should I just merge without reviewing? (tell me if you are ready to merge)
otherwise if you want my feedback on something in particular can you please comment on the commit/diff line(s) you want me to review?

@Anirban166
Copy link
Collaborator Author

it is difficult/impossible for me to review this PR because there are so many changes (103 files changed).
should I just merge without reviewing? (tell me if you are ready to merge)
otherwise if you want my feedback on something in particular can you please comment on the commit/diff line(s) you want me to review?

That's true. The changes are same for each and every html file under plots/ and posfuns/ under different plot-type directories, so you can just check any one file (rest assured, the rest are same). It is ready to be merged!

@Anirban166
Copy link
Collaborator Author

Here is the breakdown of changes just to view: (apart from deletion of the copy of estilo1.css)

The changes are same for each and every html file under plots/ and posfuns/ under different plot-type directories

Changes to such an html file:
image
image

Changes to docs/index.html:
image
image
image

Changes to files under docs/templates/:
image
image
image

@tdhock tdhock merged commit da78cec into tdhock:gh-pages Jun 17, 2021
@tdhock
Copy link
Owner

tdhock commented Jun 17, 2021

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants