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

Add "Render all markdown cells" command, or automatically render markdown #6017

Closed
maresb opened this issue Feb 21, 2019 · 11 comments · Fixed by #6029
Closed

Add "Render all markdown cells" command, or automatically render markdown #6017

maresb opened this issue Feb 21, 2019 · 11 comments · Fixed by #6029

Comments

@maresb
Copy link

@maresb maresb commented Feb 21, 2019

  1. Currently, in order to render a markdown cell in a JupyterLab notebook, one must execute the cell.
  2. Whenever the cursor enters a rendered markdown cell, it converts the cell to code for editing.
  3. Whenever the cursor exits a markdown cell, that cell always remains unrendered.

Both (1) and (2) are expected behaviors. I find (3) quite annoying since it alters the rendered-state of markdown cells simply by passing the cursor through them:
markdown
It's particularly bad when I have markdown cells interspersed with code cells. If I scroll through my notebook with the cursor, I must go back and re-execute the markdown cells while taking care not to execute the code cells. (Unless there is some trick which I'm oblivious to!!!)

I can think of a few potential alternative solutions to the issue:

  1. Render any markdown cell as markdown whenever the cursor is not inside. (Or add an option which would enable this behavior.)
  2. If keeping some markdown cells in an unrendered state is a requirement, then a "Render all markdown cells" command would suffice.
  3. Track the state of each markdown cell as rendered/unrendered, and provide some way to toggle between the two. (Implementation details would require some thought to avoid confusing users.)
@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Feb 21, 2019

Thanks for the suggestion @maresb. I think a "render all markdown cells" is a reasonable command to have in the "View" menu.

I think this would be a good issue for a new contributor to tackle. One would add a new action to render all markdown cells in actions.tsx, then add a command in the notebook extension that runs the action:

commands.addCommand(CommandIDs.run, {

This command would then be added to the menu and command palette.

@rahulpshah
Copy link
Contributor

@rahulpshah rahulpshah commented Feb 23, 2019

Hi @ian-r-rose, I was working on this issue, I realize wouldn't it be more appropriate to have this option in the Run menu. What are your thoughts on it?

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Feb 23, 2019

Thanks for taking on the issue @rahulpshah. Sure, I think that sounds reasonable.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Feb 25, 2019

I like the idea of the Render all Markdown Cells command. I think behavior 3 should remain though. It was a deliberate choice to enable people to work across multiple markdown cells at once time. If only one MD call can be edited, it become much more difficult to copy and paste text between markdown cells. But I think this new command will improve the situation being reported upon here.

@maresb
Copy link
Author

@maresb maresb commented Feb 26, 2019

Hi everyone, thanks for the quick response! The "Render all markdown cells" command makes it easy to repair the effect of running through the notebook with the cursor.

@ellisonbg, I'm confused by your comment about your use case for "behavior 3":

If only one MD call can be edited, it become much more difficult to copy and paste text between markdown cells.

It seems to me like copy-paste involves the following procedure:

  • enter the initial cell, putting it into source-editing mode
  • copy the desired code
  • exit the source cell, causing it to render
  • enter the destination cell, putting it into source-editing mode
  • paste.

This shouldn't require that both cells be in source mode simultaneously.

On the other hand, I understand that this would make comparing the source of two markdown cells difficult.

Would it be reasonable to add a setting for "Automatically render markdown" which would toggle "behavior 3" on/off? This way, when I use markdown for quick documentation, I can turn it on so that I don't have to use the new "Render All Markdown Cells" command so often. When you need to do serious markdown editing, you could turn it off.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Feb 26, 2019

Will try to clarify. The actions you describe are more or less on. However, each time there is the "enter" and "exit" there is additional actions required (either enter/esc on the keyboard, or double mouse clicks). When you are moving lots of stuff around, it roughly doubles the number of actions a user has to take. In the current state, you can do this by simply: command+C, command+V.

However, there are situations where auto-rendering would be helpful (different activites than copy/pasting). Do folks think this makes sense to put this into the View or Settings menu?

However, there is another way of doing auto-rendering that people might think this is talking about. Namely in other notebook UIs, the raw text and rendered markdown is sometimes shown at the same time (above/below or side by side). That might be a 3rd way of presenting this. We don't need to implement that for 1.0, but I want to make sure users understand what this option means.

@maresb
Copy link
Author

@maresb maresb commented Feb 27, 2019

Good point, @ellisonbg. I agree that the text I propose could be confused with an auto-preview feature. Perhaps a better name for the option would be something like "Render markdown on cell exit".

I'm still struggling to understand your point regarding additional keyboard/mouse actions required. (Sorry, I'm not trying to be difficult! We must have a misunderstanding.) As I understand, if "Render markdown on cell exit" were enabled, then exiting a cell would not require any additional clicks or key presses. Editing a markdown cell with the keyboard would work exactly the same. To edit with the mouse, one must double-click (instead of single-click) on the rendered cell. So worst case (using mouse instead of keyboard) I count a double-click in place of a single-click.

When using the keyboard, it should still be possible to copy/paste via the usual procedure: Shift-select, Ctrl+C, use arrows move the edit-cursor (possibly to another cell), Ctrl+V.

Thanks for your patience.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Feb 27, 2019

Note that to edit a rendered markdown cell, you have to double-click to change it to source, and then click again to switch the editor to edit mode and select a place to edit. This works the same in the current classic notebook.

I recently tried to make the double-click actually place the cursor, but quickly realized that unless we have a mapping of output position to source position (we don't), you don't really want to place the cursor by the double-click position.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Mar 6, 2019

@maresb I like the "Render markdown on cell exit" phrasing, but I think the capitalization would "Render Markdown on Cell Exit" based on our standard.

The double actions and mouse clicks are along the lines that @jasongrout mentions above. In many situations it isn't a big deal, but when moving lots of content around it gets really painful.

@maresb
Copy link
Author

@maresb maresb commented Mar 8, 2019

Thanks for all the feedback, and for explaining the mouse-click issue. If I understand correctly, it sounds like it may be feasible to implement a toggle setting "Render Markdown on Cell Exit", assuming that by default it is set to "off"?

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Mar 8, 2019

That sounds good to me.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants