#10825 closed enhancement (fixed)
Make gap support SAGE_SPKG_INSTALL_DOCS
Reported by: | jason | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | packages: standard | Keywords: | |
Cc: | wdjoyner@…, sal@… | Merged in: | sage-4.7.alpha1 |
Authors: | Jason Grout | Reviewers: | David Kirkby |
Report Upstream: | Reported upstream. Developers acknowledge bug. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket implements #10823 for gap. New spkg at http://sage.math.washington.edu/home/jason/spkg-docs/gap-4.4.12.p5.spkg
Attachments (1)
Change History (18)
comment:1 Changed 11 years ago by
- Status changed from new to needs_review
comment:2 Changed 11 years ago by
- Reviewers set to David Kirkby
- Status changed from needs_review to needs_work
comment:3 Changed 11 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
New spkg up which should take care of David's points.
comment:4 Changed 11 years ago by
I'm not in a position to check this now, but it would help if you could add the patch to the ticket, as not only does it making reviewing it much easier, but it is useful for others to quickly look at what changes were made.
Dave
comment:5 Changed 11 years ago by
Yep, there's a patch attached to the ticket now (for this and the other spkg tickets, as I finish them).
comment:6 follow-up: ↓ 11 Changed 11 years ago by
- Cc wdjoyner@… sal@… added
- Report Upstream changed from N/A to Reported upstream. Little or no feedback.
- Status changed from needs_review to positive_review
The GAP change is implemented fine, so positive review.
However, the syntax of the HTML in GAP's documentation is incorrect. I just looked in index.htm and this immediately struck me:
<OL> <LI><A HREF="tut/chapters.htm">Tutorial</A> <LI><A HREF="ref/chapters.htm">Reference Manual</A> <LI><A HREF="prg/chapters.htm">Programming Tutorial</A> <LI><A HREF="ext/chapters.htm">Programming Reference Manual</A> <LI><A HREF="new/chapters.htm">New Features for Developers</A> </OL>
There should be a </LI>
on the end of list items, so the first would be:
<LI><A HREF="tut/chapters.htm">Tutorial</A></LI>
but all 5 are missing, but that's obviously an upstream problem, so I have cc'ed the two people listed as the upstream contacts for GAP
comment:7 Changed 11 years ago by
I agree with you on all accounts.
comment:8 follow-up: ↓ 9 Changed 11 years ago by
From Steve Linton: "Thanks for the info. The documentation mechanisms are changing completely in the next release, and we will make sure they produce valid HTML.
Steve"
comment:9 in reply to: ↑ 8 Changed 11 years ago by
- Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.
Replying to was:
From Steve Linton: "Thanks for the info. The documentation mechanisms are changing completely in the next release, and we will make sure they produce valid HTML.
Steve"
Thank you Steve for the prompt reply, and for William for forwarding it to trac.
Dave
comment:10 Changed 11 years ago by
- Merged in set to sage-4.7.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
comment:11 in reply to: ↑ 6 Changed 11 years ago by
Replying to drkirkby:
The GAP change is implemented fine, so positive review.
However, the syntax of the HTML in GAP's documentation is incorrect. I just looked in index.htm and this immediately struck me:
<OL> <LI><A HREF="tut/chapters.htm">Tutorial</A> <LI><A HREF="ref/chapters.htm">Reference Manual</A> <LI><A HREF="prg/chapters.htm">Programming Tutorial</A> <LI><A HREF="ext/chapters.htm">Programming Reference Manual</A> <LI><A HREF="new/chapters.htm">New Features for Developers</A> </OL>
This is actually correct HTML, the </LI>
end tags are optional. So it's not a bug, even if the developers acknowledge it.
comment:12 Changed 11 years ago by
See also the W3C validator. There is an error for a misplaced </P>
but it doesn't complain about the missing </LI>
s.
comment:13 Changed 11 years ago by
huh. Thanks for the info. One more reason to like xhtml more than html, I guess!
comment:14 Changed 10 years ago by
I'm not sure what the following is supposed to do (or mean):
# Build and install docs if requested if [ "x$SAGE_SPKG_INSTALL_DOCS" = xyes ] ; then if [ $? -ne 0 ]; then echo "Error building gap docs." exit 1 fi mkdir -p $SAGE_ROOT/local/share/doc/gap cp -r src/doc/htm $SAGE_ROOT/local/share/doc/gap/html fi
Either there's a command missing to build the documentation, or we don't have to build it, in which case testing $?
there is useless, since previous errors are catched above that block. (On the other hand, whether the creation of the directory and the copy command succeeded isn't checked at all, so maybe the if
-block was supposed to be below the cp
.)
I've currently changed this to
# Build and install docs if requested: if [[ "$SAGE_SPKG_INSTALL_DOCS" = yes ]]; then # ????? if [[ $? -ne 0 ]]; then echo "Error building GAP docs." exit 1 fi mkdir -p "$SAGE_LOCAL"/share/doc/gap && cp -r src/doc/htm "$SAGE_LOCAL"/share/doc/gap/html if [[ $? -ne 0 ]]; then echo >&2 "Error copying GAP's HTML documentation." exit 1 fi fi
Btw., the description in SPKG.txt
(still) says the docs (as well as tests, which seem to be there again, too) have been removed from the spkg. The src/doc/
directory in contrast contains (subdirectories with) lots of LaTeX, HTML and probably other files.
I'm currently working on a GAP 4.4.12.p7 spkg for #7041, so perhaps leave comments (also) there.
comment:15 follow-ups: ↓ 16 ↓ 17 Changed 10 years ago by
We've talked a lot about a different way to go about building and installing the spkg docs (see #11197, for example) by having a separate spkg-install-docs script, sort of like how we have a separate spkg-check script.
I agree that the current test of $? doesn't make much sense; something is clearly missing.
comment:16 in reply to: ↑ 15 Changed 10 years ago by
Replying to jason:
We've talked a lot about a different way to go about building and installing the spkg docs (see #11197, for example)
Just wanted to cc me on that ticket, then realizing that I've already commented on it... m)
(To my excuse, this was 7 month ago.)
I agree that the current test of $? doesn't make much sense; something is clearly missing.
Well, currently the (apparently pre-built) HTML docs to get installed are there; I'm just not sure whether these are from upstream, or some Sage developer prepared them. I'll just add a note on that to SPKG.txt
; I think we can then remove the other files (LaTeX sources etc.) from the spkg, at least for the moment. (Dima was recently talking about upgrading to the almost-released GAP 4.5 anyway.)
comment:17 in reply to: ↑ 15 Changed 10 years ago by
Replying to jason:
having a separate spkg-install-docs script, sort of like how we have a separate spkg-check script.
Actually, I like that!
See comments on #10828