Opened 4 years ago
Closed 4 years ago
#25537 closed defect (fixed)
sage -rst2ipynb should provide a usefull message if rst2ipynb is not installed
Reported by: | slabbe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.3 |
Component: | scripts | Keywords: | thursdaysbdx |
Cc: | tmonteil | Merged in: | |
Authors: | Sébastien Labbé, Thierry Monteil | Reviewers: | Thierry Monteil, Sébastien Labbé |
Report Upstream: | N/A | Work issues: | |
Branch: | da7f22b (Commits, GitHub, GitLab) | Commit: | da7f22b444281e0c61253a68d929512732bcf44f |
Dependencies: | #23416 | Stopgaps: |
Description
I get:
$ sage -rst2ipynb yo.rst yo.ipynb /home/slabbe/GitBox/sage/src/bin/sage: ligne 775 : exec: rst2ipynb : non trouvé
???
Change History (15)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
When kash, polymake, etc. are not installed, the doc of sage -advanced
says:
-kash [...] -- run Sage's Kash with given arguments (not installed currently, run sage -i kash) -lisp [...] -- run Lisp interpreter included with Sage -M2 [...] -- run Sage's Macaulay2 with given arguments (not installed currently, run sage -i macaulay2) -maxima [...] -- run Sage's Maxima with given arguments -mwrank [...] -- run Sage's mwrank with given arguments -polymake [...] -- run Sage's Polymake with given arguments (not installed currently, run sage -i polymake)
comment:3 Changed 4 years ago by
- Branch set to u/slabbe/25537
- Commit set to 459e5d3891498195bc8789073ae9d1ad7d266e16
- Dependencies set to #23416
- Status changed from new to needs_review
I changed sage -advanced
for rst2ipynb
to do like the others kash
and polymake
, etc.
Last 10 new commits:
2af3bfc | #23416 : let nbconvert write file and save images
|
aa75e08 | #23416 : postprocessing
|
03e157c | Merge branch 'u/tmonteil/provide_a__sage__ipynb2rst__command' of trac.sagemath.org:sage into HEAD
|
0785497 | #23416 : command line doctest
|
5ed5ab1 | #23416 : remove depedency test to pandoc
|
7897736 | #23416 : typo
|
30806a2 | Merge branch 'u/tmonteil/provide_a__sage__ipynb2rst__command' of trac.sagemath.org:sage into HEAD
|
f935b04 | #23416 : has_pandoc doctest function
|
6e37aa2 | #23416 : update cmdline doctest
|
459e5d3 | 25537: add doc to install rst2ipynb in sage -advanced
|
comment:4 Changed 4 years ago by
- Keywords thursdaysbdx added
comment:5 Changed 4 years ago by
- Cc tmonteil added
comment:6 Changed 4 years ago by
- Reviewers set to Thierry Monteil
The code you provide does not fix the issue mentioned in the title and the description.
comment:7 Changed 4 years ago by
Yes I know. Do you know how we could give better error messages to the user when not installed? You know bash more than me...
comment:8 Changed 4 years ago by
- Branch changed from u/slabbe/25537 to u/tmonteil/25537
comment:9 Changed 4 years ago by
- Commit changed from 459e5d3891498195bc8789073ae9d1ad7d266e16 to da7f22b444281e0c61253a68d929512732bcf44f
comment:10 follow-up: ↓ 13 Changed 4 years ago by
- Reviewers changed from Thierry Monteil to Thierry Monteil, Sébastien Labbé
- Status changed from needs_review to needs_info
Do we lose something by removing the exec
part ?
comment:11 Changed 4 years ago by
ping:) please can you answer my small question before I can give positive review?
comment:12 Changed 4 years ago by
ping
comment:13 in reply to: ↑ 10 Changed 4 years ago by
Replying to slabbe:
Do we lose something by removing the
exec
part ?
The way i wrote it, i would say no. The trick with exec
is that the current process is replaced, hence after the command is finished, the further lines of the current script are not executed. This explains why, when removing the exec
i had to catch the error (to send the appropriate message) and then be sure to exit.
The only drawback i see, is that the construction is different from the other cases in the script (that still use exec).
comment:14 Changed 4 years ago by
- Status changed from needs_info to positive_review
Thanks for the clarifications.
comment:15 Changed 4 years ago by
- Branch changed from u/tmonteil/25537 to da7f22b444281e0c61253a68d929512732bcf44f
- Resolution set to fixed
- Status changed from positive_review to closed
It seems to be the normal behavior when an optional package is not installed: