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

DOC Add a button to copy code (#12512) #26666

Merged
merged 10 commits into from
Jun 23, 2023

Conversation

tuscland
Copy link
Contributor

Reference Issues/PRs

Closes #12512.

What does this implement/fix? Explain your changes.

In the documentation, add a copy button to all code blocks.
When clicked, the text in the block is filtered in order to remove prompts (>>>) and output lines. Text without prompt is copied as-is.

Any other comments?

  • The sphinx_copybutton extension was used.
  • The default style draws an outline around the hovered button. To make it look better, a CSS declaration was added.

@github-actions
Copy link

github-actions bot commented Jun 22, 2023

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: cdce63d. Link to the linter CI: here

@tuscland tuscland changed the title DOC Add a button to copy code. Prompt and output lines are omitted. DOC Add a button to copy code (#12512) Jun 22, 2023
Copy link
Member

@betatim betatim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a nice change. The copybutton sphinx extension is quite widely used as well.

One thing to try and find out from the archives: what was the thinking "back then" to add a button that hides the prompts, instead of having one that copies the text to the clipboard? Is it an accident of history or was there some specific thinking about this aproach (hide and then let the user copy themselves).

@tuscland
Copy link
Contributor Author

One thing to try and find out from the archives: what was the thinking "back then" to add a button that hides the prompts, instead of having one that copies the text to the clipboard? Is it an accident of history or was there some specific thinking about this aproach (hide and then let the user copy themselves).

I believe the "hide prompts" button allowed for more controlled copy & paste user experience, because when you copy a block of code and paste it all, you might miss intermediate results.

Copy link
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition! LGTM as it is. Thanks @tuscland.

@ArturoAmorQ ArturoAmorQ added the dependencies Pull requests that update a dependency file label Jun 23, 2023
@betatim betatim merged commit f1026c0 into scikit-learn:main Jun 23, 2023
29 checks passed
@betatim
Copy link
Member

betatim commented Jun 23, 2023

If someone can find a way to customise the "copy" label that appears on the left when you hover the button (not the bottom label that comes from the browser) that would be great. We could use it to make it clearer to the (sceptical) user what exactly will be copied (only the inputs, not the outputs).

Screenshot 2023-06-23 at 14 01 28

@tuscland
Copy link
Contributor Author

If someone can find a way to customise the "copy" label that appears on the left when you hover the button (not the bottom label that comes from the browser) that would be great. We could use it to make it clearer to the (sceptical) user what exactly will be copied (only the inputs, not the outputs).

It would require to propose a change to sphinx-copybutton localized messages:
https://github.com/executablebooks/sphinx-copybutton/blob/4a8050dea36536b005d3e757b1ac09886174f634/sphinx_copybutton/_static/copybutton.js_t#L5

@ArturoAmorQ
Copy link
Member

It seems that this PR still has an issue. The copy button ignores lines that start by .... For instance, the following code:

>>> train_scores, test_scores = validation_curve(
...     logistic_regression, X, y, param_name=param_name, param_range=param_range
... )

ends up copying only

>>> train_scores, test_scores = validation_curve(

@betatim
Copy link
Member

betatim commented Jun 27, 2023

I'll try and make a PR to repair this or revert this change.

@tuscland
Copy link
Contributor Author

I understand ... is a continuation of the prompt, therefore we should use a different configuration for the sphix_copybutton plugin. Currently we have:

copybutton_prompt_text = ">>> "

@betatim as per the documentation, we could try this instead:

copybutton_prompt_text = r">>> |\.\.\. |\$ |In \[\d*\]: | {2,5}\.\.\.: | {5,8}: "
copybutton_prompt_is_regexp = True

REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy button for code block
3 participants