Opened 2 years ago
Closed 2 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:  sage8.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)  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 2 years ago by
comment:2 Changed 2 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 2 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 2 years ago by
 Keywords thursdaysbdx added
comment:5 Changed 2 years ago by
 Cc tmonteil added
comment:6 Changed 2 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 2 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 2 years ago by
 Branch changed from u/slabbe/25537 to u/tmonteil/25537
comment:9 Changed 2 years ago by
 Commit changed from 459e5d3891498195bc8789073ae9d1ad7d266e16 to da7f22b444281e0c61253a68d929512732bcf44f
comment:10 followup: ↓ 13 Changed 2 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 2 years ago by
ping:) please can you answer my small question before I can give positive review?
comment:12 Changed 2 years ago by
ping
comment:13 in reply to: ↑ 10 Changed 2 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 2 years ago by
 Status changed from needs_info to positive_review
Thanks for the clarifications.
comment:15 Changed 2 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: