Add syntax highlighting to pari_jupyter kernel

[vinklein]

Add a basic syntax highlith using codemirror "SimpleMode"

linked to OpenDreamkit issue #175

[vinklein]

@jdemeyer Are you ok with this PR or have you any doubt ?

[jdemeyer]

I haven't looked yet. But I do intend to do that.

[jdemeyer]

Where did you get the names in the var builtin list? It would be better to auto-generate that list. There are also many names (starting with an underscore) which are not valid GP functions.

[jdemeyer]

The installation is broken:

error: [Errno 13] Permission denied: '/usr/local/share/jupyter'

I don't know where it got this path (/usr/local/share) from, but it's not where I actually installed the PARI kernel. The extension should be installed in the same place where the kernel is installed.

[jdemeyer]

See jupyter/notebook#1706 for the installation issue.

[jdemeyer]

We should really avoid notebook.nbextensions.install_nbextension and instead install it manually.

[jdemeyer]

We should really avoid notebook.nbextensions.install_nbextension and instead install it manually.

It seems that you doing both: you use data_files to install the files manually and then you also use notebook.nbextensions.install_nbextension. Simply removing the latter fixes the installation for me.

[jdemeyer]

The highlighting itself looks good. No complaints there.

[jdemeyer]

To accept this pull request, the minimal changes required are:

1. Remove the invalid builtin names
2. Either document how you obtained the list of builtin names or auto-generate that list
3. Remove the install_nbextension() call

[vinklein]

The function builtin list has been generated from file src/desc/pari.desc (pari 2.9.3 sources).
Minus function where Class: gp2c. Some other function have been manually removed from the list.

• Do you think pari.desc is the good file to generate this list or do you have a better alternative ?
• If we keep pari.desc as a base to generate the function list should we remove gp2c_internal functions plus all the functions beginning with '_' to have a proper list ?

[vinklein]

Just removing install_nbextension() will not work because his role is also to add gp-mode as an extension to the notebook.json file. Without this the extension will not be loaded (I think it works in your case because pip uninstall doesn't modify the json file). I will look to add manually gp-mode to the notebook.json file.

[jdemeyer]

Do you think pari.desc is the good file to generate this list or do you have a better alternative ?

Yes.

If we keep pari.desc as a base to generate the function list should we remove gp2c_internal functions plus all the functions beginning with '_' to have a proper list ?

I think that you just need to take the functions with Class: basic.

[jdemeyer]

Just removing install_nbextension() will not work because his role is also to add gp-mode as an extension to the notebook.json file.

No, I think that is done by the enable_nbextension call. I just tested removing the install_nbextension call and it seems to work.

[vinklein]

My notebook.json contain

{
  "load_extensions": {
    "gp-mode/main": true,
    "singular-mode/main": true,
    "gap-mode/main": true
  }
}

maybe the format is version dependent.

[vinklein]

I think that you just need to take the functions with Class: basic.

I think you mean minus the operators (-_, %#, etc)

No, I think that is done by the enable_nbextension call

I will test that.

[vinklein]

The new list is with Class: basic without "operator" like function.

[jdemeyer]

I think you mean minus the operators (-_, %#, etc)

I would say: minus anything that starts with an underscore.

I am assuming that you used some kind of script to generate this list from pari.desc, could you add this script such that we can easily update the javascript in the future.

[vinklein]

Yes but it's more a kind of messy one shot script right now (with french comments inside), i will modify and complete it before adding it to a pari_jupyter/tools directory

[vinklein]

generation script has been added

[jdemeyer]

I merged this, adding some changes. I decided to also remove enable_extension() for now because it doesn't always do the right thing either. See jupyter/notebook#2824

[vinklein]

I like the improvements.
But your current version don't work, because there is missings coma in the var builtin list (at each end of line)

var builtin = createBuiltinRegularExpression([
    'Catalan', 'Col', 'Colrev', 'Euler', 'I', 'List', 'Map', 'Mat', 'Mod', 'O'
    'Pi', 'Pol', 'Polrev', 'Qfb', 'Ser', 'Set', 'Str', 'Strchr', 'Strexpand'
    ...

should be

var builtin = createBuiltinRegularExpression([
    'Catalan', 'Col', 'Colrev', 'Euler', 'I', 'List', 'Map', 'Mat', 'Mod', 'O',
    'Pi', 'Pol', 'Polrev', 'Qfb', 'Ser', 'Set', 'Str', 'Strchr', 'Strexpand',
    ...

[vinklein]

And removing enable_extension don't work for me either for a first installation.
My notebook.json doesn't contain "gp-mode/main": true and therefore the mode is not loaded.

~$ jupyter --version
4.4.0
~$ jupyter notebook --version
5.2.2

[jdemeyer]

I know, you have to manually enable the extension.

[jdemeyer]

But your current version don't work, because there is missings coma in the var builtin list

Oops. Fixed.

[jdemeyer]

It turns out that Jupyter notebook version 5.3 can automatically enable extensions. I just fixed the installation of pari_jupyter to do that.

[vinklein]

It's nice ! It helps to keep a clean setup.py.