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

pretty unicode banner #14733

Closed
fchapoton opened this issue Jun 12, 2013 · 43 comments
Closed

pretty unicode banner #14733

fchapoton opened this issue Jun 12, 2013 · 43 comments

Comments

@fchapoton
Copy link
Contributor

Using unicode, one can get the banner to look like that:

┌────────────────────────────────────────────────────────────────────┐
│ Sage Version 5.8, Release Date: 2013-03-15                         │
│ Type "notebook()" for the browser-based notebook interface.        │
│ Type "help()" for help.                                            │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

Apply:

Depends on #14559

CC: @sagetrac-tmonteil

Component: user interface

Keywords: banner, unicode

Author: Frédéric Chapoton

Reviewer: Volker Braun, William Stein

Merged: sage-5.12.beta1

Issue created by migration from https://trac.sagemath.org/ticket/14733

@fchapoton fchapoton added this to the sage-5.11 milestone Jun 12, 2013
@fchapoton
Copy link
Contributor Author

Dependencies: #14559

@fchapoton
Copy link
Contributor Author

Attachment: trac_14733_banner_unicode.patch.gz

@vbraun
Copy link
Member

vbraun commented Jun 13, 2013

comment:2

Since we are already going all-out, how about color instead of a second box for the warning.

@fchapoton
Copy link
Contributor Author

comment:3

I do not know how to put color, but this is a good idea.

I would put the bold box in red, or the bold box and its content in red.

@fchapoton
Copy link
Contributor Author

comment:4

here is color version

for the bot:

apply trac_14733_banner_unicode_color.patch

@fchapoton
Copy link
Contributor Author

comment:5

apply trac_14733_banner_unicode_color.patch

@fchapoton
Copy link
Contributor Author

comment:6

Is there somebody interested to review this patch ?

apply trac_14733_banner_unicode_color.patch

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 5, 2013

comment:7

Hmmmmmmmmm O_o

I don't know what is happening but applying this patch makes absolutely NO difference to the banner I see when Sage starts O_o

Nathann

@vbraun
Copy link
Member

vbraun commented Jul 5, 2013

comment:8

The banner you see on startup is cached output from the sage-sdist script.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 5, 2013

comment:9

Oh. And how can I test it then ? O_o

Nathann

@vbraun
Copy link
Member

vbraun commented Jul 6, 2013

comment:10

I manually updated local/bin/sage-banner and get the following two doctest errors:

$ sage -t --long devel/sage/sage/tests/cmdline.py  # 1 doctest failed
Running doctests with ID 2013-07-05-21-44-50-a5339967.
Doctesting 1 file.
sage -t --long devel/sage/sage/tests/cmdline.py
**********************************************************************
File "devel/sage/sage/tests/cmdline.py", line 185, in sage.tests.cmdline.test_executable
Failed example:
    out.find(version()) >= 0
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   1 of 205 in sage.tests.cmdline.test_executable
    [204 tests, 1 failure, 17.97 s]
----------------------------------------------------------------------
sage -t --long devel/sage/sage/tests/cmdline.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 18.0 seconds
    cpu time: 0.2 seconds
    cumulative wall time: 18.0 seconds
$ sage -t --long devel/sage/sage/misc/trace.py  # 1 doctest failed
Running doctests with ID 2013-07-05-21-45-09-11b2ac64.
Doctesting 1 file.
sage -t --long devel/sage/sage/misc/trace.py
**********************************************************************
File "devel/sage/sage/misc/trace.py", line 61, in sage.misc.trace.trace
Failed example:
    print s.before[s.before.find('-'):]
Expected:
    ---...
    ipdb> c
    2 * 5
Got:
    -06-21                  │
    │ Type "notebook()" for the browser-based notebook interface.        │
    │ Type "help()" for help.                                            │
    └────────────────────────────────────────────────────────────────────┘
    <CSI-31m>┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
    ┃                                                                    ┃
    ┃ Warning: this is a prerelease version, and it may be unstable.     ┃
    ┃                                                                    ┃
    ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛<CSI-0m>
    trace('print factor(10)'); print 3+97
    s<CSI-?1034h>
    c
    <CSI-0;34m>sage: <CSI-0m>trace('print factor(10)'); print 3+97
    > <string>(1)<module>()
    
    ipdb> s
    --Call--
    > /home/vbraun/opt/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/rings/arith.py(2314)factor()
       2313 
    -> 2314 def factor(n, proof=None, int_=False, algorithm='pari', verbose=0, **kwds):
       2315     """
    
    ipdb> c
    2 * 5
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of   9 in sage.misc.trace.trace
    [8 tests, 1 failure, 1.28 s]
----------------------------------------------------------------------
sage -t --long devel/sage/sage/misc/trace.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 1.3 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 1.3 seconds

@vbraun
Copy link
Member

vbraun commented Jul 6, 2013

comment:11

Also, wouldn't it be better to have no empty lines in the warning box? Having one box with and one box without the blank lines really triggers my OCD ;-)

┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

@fchapoton
Copy link
Contributor Author

comment:12

Attachment: trac_14733_banner_unicode_color.patch.gz

apply trac_14733_banner_unicode_color.patch

  • I have remove the empty lines in the warning part
  • I have solved the problem with trace.py
  • the problem with cmdline.py is also there without the patch, namely sage -v on a bare 5.11.beta3 does not display anything

@vbraun
Copy link
Member

vbraun commented Jul 6, 2013

comment:13

If you look at $SAGE_ROOT/spkg/bin/sage then you'll see that sage -v gets the version out of the cached sage-banner. And the regex doesn't succeed any more in extracting the version information from the new banner. It works fine on a vanilla sage-5.11.beta3 with unmodified sage-banner.

@fchapoton
Copy link
Contributor Author

comment:14

ok, you are right. But what to do ?
It seems to be sufficient to replace the line 343 in $SAGE_ROOT/spkg/bin/sage by

    exec sed -n -e '/Version/s/^[ │]\{1,\}//;/Version/s/[ │]\{1,\}$//p' \

But maybe one has to declare the unicode encoding in the first line of this file too ? I do not know is bash is friendly with unicode.

And I have no idea how to deal with this kind of change, I have only worked with patches so far, never with any spkg.

@vbraun
Copy link
Member

vbraun commented Jul 6, 2013

comment:15

No spkg changes required, you just need to write a patch against the $SAGE_ROOT repository

@fchapoton
Copy link
Contributor Author

comment:16

ok, so here is a patch, but still I am not sure if bash/sed can handle unicode cleanly.

@vbraun
Copy link
Member

vbraun commented Jul 6, 2013

comment:17

Sed just operates on byte strings. I would prefer to make the regex less likely to break by just matching for the part that we want, not the part that we don't want:

sed -n -e 's/.*\(Version.*Date: [0-9-]*\).*/\1/p'

@fchapoton

This comment has been minimized.

@fchapoton
Copy link
Contributor Author

comment:18

ok, I agree that this is better. Here is a modified patch.

apply trac_14733_banner_unicode_color.patch trac_14733_version_regexp.patch

@fchapoton
Copy link
Contributor Author

comment:19

well, the bot does not seem to understand that one patch is for the SAGE_ROOT directory..

@vbraun
Copy link
Member

vbraun commented Jul 6, 2013

comment:20

Yes, thats a limitation of the patchbot

@vbraun
Copy link
Member

vbraun commented Jul 7, 2013

comment:21

Sorry, I forgot to start the regex with "Sage Version". The cmdline.py test still fails because it expects "Sage Version..." instead of "Version..."

@fchapoton
Copy link
Contributor Author

Attachment: trac_14733_version_regexp.patch.gz

@fchapoton
Copy link
Contributor Author

comment:22

here is a corrected patch

@vbraun

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Jul 7, 2013

Reviewer: Volker Braun

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Jul 7, 2013

comment:24

I wonder whether coloured UTF-8 passes easily on most terminal, in particular quite a lot of users use sage through a ssh connection. I didn't test further, but on xvt (Debian wheezy) i got:

sage: sage.misc.banner.banner()
ââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââ
â Sage Version 5.10.rc0, Release Date: 2013-05-29                    â
â Type "notebook()" for the browser-based notebook interface.        â
â Type "help()" for help.                                            â
ââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââ
ââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââ
â                                                                    â
â Warning: this is a prerelease version, and it may be unstable.     â
â                                                                    â
ââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââââ

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Jul 8, 2013

comment:25

I tried with more terminal emulators (still using Debian wheezy), and

  • kterm does not show the boxes (not a serious problem)
  • Eterm leads to a very bad output
  • aterm, mrxvt-mini and mrxvt lead to:
$ sage
+--------------------------------------------------------------------+
| Sage Version 5.10.rc0, Release Date: 2013-05-29                    |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
+--------------------------------------------------------------------+
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
sage: sage.misc.banner.banner()
����������������������������������������������������������������������
� Sage Version 5.10.rc0, Release Date: 2013-05-29                    �
� Type "notebook()" for the browser-based notebook interface.        �
� Type "help()" for help.                                            �
����������������������������������������������������������������������
����������������������������������������������������������������������
�                                                                    �
� Warning: this is a prerelease version, and it may be unstable.     �
�                                                                    �
����������������������������������������������������������������������
sage:

Acces via various ssh connection (especially Putty on windows connecting to various server settings) should be tested as well.

While i find this banner very pretty, i do not have any idea how to fix this encoding issue. Any idea ?

@vbraun
Copy link
Member

vbraun commented Jul 8, 2013

comment:26

Given that Sage is gravitating towards UTF-8 encoded source files (and, by extension, docstrings), I would argue that it is a feature that we make it immediately and unambiguously clear to the user that his terminal does not support utf8. There is no API to query the terminal whether it is utf8 capable, you essentially can only check it by displaying some non-ascii characters. Also, even if you don't have a UTF-8 capable terminal you can still use Sage, only the banner looks funky.

In particular, nobody should be using the following any more at this time:

  • rxvt (and various older *xvt clones) does not support unicode, it has been forked into rxvt-unicode (a.k.a. urxvt).
  • Eterm does not support unicode. Some distros (debian) apply an unofficial patch, but your mileage may vary.

Plain xterm, urxvt, and gnome-terminal work for me.

Ssh will happily pass UTF-8 through, it is the terminal at the other end that needs to be able to display it.

@vbraun
Copy link
Member

vbraun commented Jul 8, 2013

comment:27

I've posted a RFC to sage-devel: https://groups.google.com/d/msg/sage-devel/GbaC9yeicAY/GHYcXAoxxP8J

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 9, 2013

comment:28

This is somewhat unrelated comment, but the following may be considered by the patch writer.

The first line "Sage Version 5.8, Release Date: 2013-03-15" may be simplified to

"Sage 5.8, released on 2013-03-15"

I think currently "Version" is unduly prominent and "Date" seems redundant. Too much minimalistic? :-)

@fchapoton
Copy link
Contributor Author

comment:29

well, there seems to be no much hope to get that patch in sage, so let me keep the patch as it is for the moment.

@fchapoton
Copy link
Contributor Author

comment:30

and let me keep this as need review in the present state, just in case somebody is interested

@williamstein
Copy link
Contributor

comment:31

Replying to @fchapoton:

well, there seems to be no much hope to get that patch in sage, so let me keep the patch as it is for the moment.

I used the patch, and I think it is beautiful. I completely disagree with the comments such as " I dislike the UTF-8 banner because it looks too nice." and "Keep the banner short and simple ; people don't come to sage to enjoy that sight!" Clean beauty is exactly what people (at least me!) want in software. The banner in Sage right now, which I probably wrote (?), looks frankly ugly and like a hack, compared to the one on this patch.

Also, UTF is clearly the future of strings, having native default support in modern interpreters, editors, etc., and also being critical to supporting users who aren't using English.

This patch is along the same lines as the recent inclusion of a nice color prompt (thanks volker), in that it makes Sage prettier and more pleasant to use.

So my strong vote for this ticket. Moreover, I like it so much I'll be henceforth applying it to the standard system-wide version of Sage at https://cloud.sagemath.com, even if it doesn't get into Sage. In particular, I disagree with " If you're doing something that requires nice output you should probably be using a notebook interface anyways" -- since the terminal interface is part of the notebook interface now, and it must look nice.

@fchapoton
Copy link
Contributor Author

comment:32

Thanks William for your strong opinion. I hope that somebody will soon find the time to review this patch.

@jdemeyer
Copy link

jdemeyer commented Aug 7, 2013

Changed reviewer from Volker Braun to Volker Braun, William Stein

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 7, 2013
@williamstein
Copy link
Contributor

comment:35

Replying to @nathanncohen:

Oh. And how can I test it then ? O_o

Apply the two patches, start sage and type banner() at the prompt. Make it the banner by typing sage -c "banner()" > local/bin/sage-banner.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Aug 16, 2013

comment:36

Wow. Nice ! :-)

Nathann

@jdemeyer
Copy link

Merged: sage-5.12.beta1

@nthiery
Copy link
Contributor

nthiery commented Aug 21, 2013

comment:38

This probably causes sage -version to hang on MacOS X; see #15072.

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

No branches or pull requests

6 participants