Minor fixes and improvements on frontend extension

[mahendrapaipuri]

This is follow up of #1163 that mainly attempts to fix remarks made here.

@parmentelat @mwouts Do you think settings page has better readability now?

The thing is Settings use rjsf behind the scenes and we cannot use text formatting like bold, italic, etc. Even if we use HTML tags, they will be escaped and rendered literally. So, I used a null JSON type to show message about browser refresh at the end in a separate line.

@mwouts Before we merge this, we need to update the documentation unless you are working on it. I can update the screenshots and basic documentation of extension and maybe later you can expand on it. What do you think? But before its better we agree on settings UI.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (2bf03e6) 96.74% compared to head (60ab3ee) 97.73%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1178 +/- ## ========================================== + Coverage 96.74% 97.73% +0.98% ========================================== Files 29 29 Lines 4456 4456 ========================================== + Hits 4311 4355 +44 + Misses 145 101 -44 ``` | [Flag](https://app.codecov.io/gh/mwouts/jupytext/pull/1178/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | Coverage Δ | | |---|---|---| | [external](https://app.codecov.io/gh/mwouts/jupytext/pull/1178/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `75.05% <ø> (ø)` | | | [functional](https://app.codecov.io/gh/mwouts/jupytext/pull/1178/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `88.56% <ø> (ø)` | | | [integration](https://app.codecov.io/gh/mwouts/jupytext/pull/1178/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `77.31% <ø> (ø)` | | | [unit](https://app.codecov.io/gh/mwouts/jupytext/pull/1178/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `66.56% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[mwouts]

Hi @mahendrapaipuri , thank you for being back to this! Well, if I can request a few more changes, this is what I think we could do:

1. Move the New Text Notebook sub-menu under File/New, create a Jupytext sub-menus with the rest of the current Jupytext Menu, as in the previous Jupytext Menu (well, if you agree that these operations are well fitted in the File menu?)
2. I like the configuration as it is on main. I think the introductory paragraph is clear enough. If I were to change anything, I would replace kernel variants by kernel languages (since we iterate over languages rather than kernels right?). And I do not think that we need to be able to configure the name for the new launcher entry (the default, 'Jupytext', seems perfect to me!)
3. After that yes indeed we will have to update the screenshots in the documentation. I was wondering if it would make sense to take the screenshots from the automated tests?
4. Yes we could remove the New undefined command (but I guess that is already in the current PR?)

BTW I have released a RC as of yesterday: https://pypi.org/project/jupytext/1.16.0rc0/. So far everything works great! I plan to do the actual release over the next week-end if that is fine with you.

[mahendrapaipuri]

Move the New Text Notebook sub-menu under File/New, create a Jupytext sub-menus with the rest of the current Jupytext Menu, as in the previous Jupytext Menu (well, if you agree that these operations are well fitted in the File menu?)

I see that you would like to have the legacy UI. I thought having a dedicated Jupytext menu can improve the visibility accessibility. I agree moving New Text Notebook sub-menu under File>New is logical but I feel that rest of Jupytext sub-menus like pairing, metadata, FAQ, etc are not well suited for File menu. Anyways you know the user base more than me and if you think users will find it more easy having the Jupytext menu under File, we can move them.

I like the configuration as it is on main. I think the introductory paragraph is clear enough. If I were to change anything, I would replace kernel variants by kernel languages (since we iterate over languages rather than kernels right?). And I do not think that we need to be able to configure the name for the new launcher entry (the default, 'Jupytext', seems perfect to me!)

I agree that introductory paragraph has clear information. But I tend to agree with @parmentelat as well as impatient users do not tend to read dense text (that includes me. hahah!). Unfortunately, we cannot really format the text to split in multiple lines, so, I tried to modify it to make it an easy read. What do you think?

About the kernel variants, we do iterate over kernels and and we only use languages to get necessary metadata of kernel. So, if a user installs two variants of bash kernel, Jupytext menu will add both variants although both use shell language. Does it make sense? But I would trust you more on wording than myself (living in France does not help my English. Hahah!).

After that yes indeed we will have to update the screenshots in the documentation. I was wondering if it would make sense to take the screenshots from the automated tests?

This is a good idea. We just need to add link checker on md files in CI to ensure we do not have any broken links when we rename UI test files.

Yes we could remove the New undefined command (but I guess that is already in the current PR?)

Yes, this is taken care of.

BTW I have released a RC as of yesterday: https://pypi.org/project/jupytext/1.16.0rc0/. So far everything works great! I plan to do the actual release over the next week-end if that is fine with you.

That's great news. Congrats!! Once we agree, the changes the changes we are talking about should not take too much time. I can quickly update the PR for you to review.

[parmentelat]

I gave a quick try with commit 1634da25

I agree that having the 'New Text Notebook' submenu in the File submenu right under New would be a better fit, as this is the place where people will look first; the rest of the Jupytext submenu is fine like it is now; this is in an ideal world, if it's too complex to implement the current setup is fine too :)

I also agree with @mwouts that the ability to configure the 'Category' in settings brings no added value and we should get rid of it (or else, at least mention this is about the launcher entry)

apart from that the settings pane is fine now, shorter to read, and I like that the warning on reloading comes last

thanks a lot again @mahendrapaipuri for your patient work :)

[mahendrapaipuri]

Alrighty! Cheers @parmentelat for testing the patch.

Here are the latest changes

• Moved New Text Notebook under File and left rest in Jupytext menu
• Removed Category Settings config
• Using screenshots from tests in docs

@mwouts @parmentelat Let me know what do you think!!

[parmentelat]

yes !! that looks just perfect to me :)

all that remains now - food for another time I mean ;-) would be to have the 'right' kernel selected by default I was doing my test at 100 mph and I clicked enter and tried to write some javascript in a .js, but the kernel was python... really no big deal of course :)

as far as I am concerned we are a go 👍

[mwouts]

Awesome! Thank you both again. I might not have much time for the rest of this week but this is really promising.

About the kernel variants, we do iterate over kernels and and we only use languages to get necessary metadata of kernel. So, if a user installs two variants of bash kernel, Jupytext menu will add both variants although both use shell language. Does it make sense? But I would trust you more on wording than myself (living in France does not help my English. Hahah!).

Oh that's an interesting point! In the menu we just use the language and extension, is this correct? Imagine that I have 12 Python kernels, I'd prefer not to get 12 entries for Python percent notebooks? (I think I saw just one when I tested).

"All the available kernel variants will be added to launcher and jupytext menu"

Provided that my understanding above is correct, I would say that we do not even need to mention that. Most users will only have Python installed. And the other ones will have noticed that their favourite language is already available in the New Text Notebook menu.

Thanks for updating the docs too, I like the automated screenshots!!

[mahendrapaipuri]

Oh that's an interesting point! In the menu we just use the language and extension, is this correct? Imagine that I have 12 Python kernels, I'd prefer not to get 12 entries for Python percent notebooks? (I think I saw just one when I tested).

Oh, yes! You are right!! Adding all available kernel variants was my first implementation and then after our discussions we have changed it to pick up kernel languages. So, I was wrong in my previous comment. If there are multiple kernels with same language, only the last one that appears in kernelspecs will be picked. I have also made a small modification to use generic language name instead of kernelspec name in launcher to make Jupytext entries kernel agnostic.

Provided that my understanding above is correct, I would say that we do not even need to mention that. Most users will only have Python installed. And the other ones will have noticed that their favourite language is already available in the New Text Notebook menu.

Implemented this change in the latest commit.

New additions:

• I have added a new optional dependency for UI tests and installing Calysto bash kernel to ensure that extension works with non Python kernels
• Added a new UI test that creates bash text notebook and updated snapshots

[mwouts]

Thank you again @mahendrapaipuri ! Yep the settings page will be even lighter this way, and still as useful.

I have been thinking more about the location of the Jupytext Menu, and yes I am afraid I do miss the previous location (everything under File). My arguments for asking that are the following:

1. Pairing a notebook to another format is a File operation, since it decides to what set of files the notebook will be written.
2. I have a preference for Jupytext being discrete and impacting the users as little as possible. In some cases Jupytext is installed in an environment used by multiple users, and it should not distract the users that have not made an active choice in getting it.
3. 'Jupytext' is not a standard set of operations that I would expect in an application menu.

Happy to chat more about that! And looking forward to integrating this final touch!!

[mahendrapaipuri]

Pairing a notebook to another format is a File operation, since it decides to what set of files the notebook will be written.

Agree, this is a valid argument.

I have a preference for Jupytext being discrete and impacting the users as little as possible. In some cases Jupytext is installed in an environment used by multiple users, and it should not distract the users that have not made an active choice in getting it.

Alright! I see what you mean. I tried to redo the menu items like in the legacy notebook extension.

Let me know what do you think. You can check snapshots for new UI of the menu.

[github-actions[bot]]

Thank you for making this pull request.

Did you know? You can try it on Binder: or .

Also, the version of Jupytext developed in this PR can be installed with pip:

pip install git+https://github.com/mahendrapaipuri/jupytext.git@imrpovs_extension

(this requires nodejs, see more at Developing Jupytext)

[mwouts]

Hey @mahendrapaipuri , sorry for the repeated updates, I am trying to get the comment PR action to work on forks, and it's not obvious :smile: I've had to use pull_request_target and now I see that the repository is not what I think it would be...

(I have opened https://github.com/thollander/actions-comment-pull-request/issues/324)

[mahendrapaipuri]

@mwouts You can use ${{ github.event.pull_request.head.repo.full_name }} for the origin repo name of PR and ${{ github.event.pull_request.head.ref }} for the branch of origin repo.

Check jupyterlab-gitlab repo on how it is being done. Here it is in action

[mwouts]

That's really impressive @mahendrapaipuri , you have the answer to everything!! Thanks :pray:

[mwouts]

Great work again! Thank you @mahendrapaipuri .