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

[ENH] add sphinx-copybutton extension #615

Merged
merged 10 commits into from
Jul 20, 2021
Merged

[ENH] add sphinx-copybutton extension #615

merged 10 commits into from
Jul 20, 2021

Conversation

sappelhoff
Copy link
Contributor

@sappelhoff sappelhoff commented Nov 19, 2020

closes #75

based on #75 (comment), this PR (through the build artifacts) will allow us to practically inspect how using "sphinx-copybutton" may work.

@welcome
Copy link

welcome bot commented Nov 19, 2020

Thank You Banner (Image: CC-BY license, The Turing Way Community, & Scriberia. Zenodo. http://doi.org/10.5281/zenodo.3332808) Wohoo! 🎉 Thanks for opening your first pull request to the DataLad Handbook! 😎 We really appreciate your time and effort to contribute to the project - you're amazing! 👏

@sappelhoff
Copy link
Contributor Author

For example, copying this cell (screenshot of this):

image

gives me:

cd ../../
cat << EOT >> notes.txt
datalad save -m "Add note on datalad clone"

... all lines prefixed with $ are found, stripped of that prefix, and returned

But copying this cell (screenshot of that):

image

gives me the whole cell, because no $ prefix was found:

# enter your home directory using the ~ shortcut
% cd ~
% git config --global --add user.name "Bob McBobFace"
% git config --global --add user.email bob@example.com

we could also use a regex to configure more than one prefix we want to "detect". I think this works very nicely :-) WDYT?

@sappelhoff sappelhoff changed the title enh: add sphinx-copybutton extension [ENH] add sphinx-copybutton extension Nov 19, 2020
Copy link
Contributor

@adswa adswa left a comment

Choose a reason for hiding this comment

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

Ha, nice! Thanks for the addition, @sappelhoff. I tried it out locally, and it works very well for single lines of code with a $ prefix.

I've toyed around with the regex for a bit locally, trying to make here docs or multi-line commands work, but so far I've failed with one edge-case-problem after the other ;-) I'm happy to at least make the single line case work much easier with this addition.
Could you please make a small note mentioning the copy-button in docs/intro/narrative.rst about copy-pasting code?
And also please feel free to add yourself to the zenodo.json file :)

@adswa
Copy link
Contributor

adswa commented Nov 19, 2020

hey @all-contributors please add @sappelhoff for infra, tool

@allcontributors
Copy link
Contributor

@adswa

I've put up a pull request to add @sappelhoff! 🎉

@sappelhoff
Copy link
Contributor Author

Could you please make a small note mentioning the copy-button in docs/intro/narrative.rst about copy-pasting code?

👍

I tried to use the icon in the intro as an approximation of the "copybutton" ... it'd be best perhaps to use the actual copybutton icon, but it may also be overkill.

Do you have a link to the examples that currently don't work?

@adswa
Copy link
Contributor

adswa commented Nov 20, 2020

Do you have a link to the examples that currently don't work?

Anything that is a multi-line command, split with \ such as

   $ datalad download-url http://www.tldp.org/LDP/Bash-Beginners-Guide/Bash-Beginners-Guide.pdf \
     --dataset . \
     -m "add beginners guide on bash" \
     -O books/bash_guide.pdf

in the end of the second section, or any of the here documents (the first one is in the third section) where the text that gets re-directed into a file is not prefixed with a $

$ cat << EOT > notes.txt
   One can create a new dataset with 'datalad create [--description] PATH'.
   The dataset is created empty

   EOT

FTR: I've opened a feature request to copy-button: executablebooks/sphinx-copybutton#105.

@sappelhoff
Copy link
Contributor Author

I see, I didn't know what a "here document" was until now :-)

thanks for opening the feature request!

@adswa
Copy link
Contributor

adswa commented Nov 20, 2020

Its ancient, but super convenient to write formatted text into a file from the terminal :-) I'd say we wait for a response on the feature request until proceeding. I'll cherry-pick 6d8cabc into master, later, though, just to have you in zenodo.json when we make the next release.

@sappelhoff
Copy link
Contributor Author

I'd say we wait for a response on the feature request until proceeding.

agreed, it'd be great to support these two types of code snippets as well.

Thanks for the cherry-picking. Good thing I made a separate commit for that ;-)

@sappelhoff
Copy link
Contributor Author

FTR: I've opened a feature request to copy-button: executablebooks/sphinx-copybutton#105.

Unfortunately we didn't get much input so far, but I took some time and prepared a PR that may suffice for our needs: executablebooks/sphinx-copybutton#126

Copy link
Contributor Author

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Hi @adswa I added the requirement to sphinx-copybutton and my PR just got merged. I now updated this PR, and I think the line-continuation and here-document cells now work like we want them to work.

I'd appreciate a review! Below two links to test the copybutton in the RTD build artifact:

@adswa
Copy link
Contributor

adswa commented Jun 14, 2021

This is fantastic @sappelhoff , thank you so much for your efforts! I apologize in advance for a delay, a number of deadlines loom over me. It looks good on first sight, an I hope I can get to try it out and comment more within this week :)

@adswa
Copy link
Contributor

adswa commented Jul 20, 2021

This is so cool! ❤️ Apologies for getting to this with such a delay :( Again one of these things that would have been done in 5 minutes had I just not pushed it at the end of my todo-list ... 🤦

@mih, @yarikoptic check this out! It works flawlessly!

@adswa adswa merged commit eedf29e into datalad-handbook:master Jul 20, 2021
@sappelhoff sappelhoff deleted the copybutton branch July 20, 2021 09:45
@sappelhoff
Copy link
Contributor Author

No worries, thanks for maintaining this book! I am happy to see this in :-)

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

Successfully merging this pull request may close these issues.

improve the way code is displayed
2 participants