Add regexp prompt matching ( style and testing) #82

chrisjsewell · 2020-06-12T23:21:09Z

Right, in preparation for #52, I've whipped this package into shape:

Added pre-commit configuration and CI style testing
Extracted the "DOM independent" JS code to a separate file, in order to
Add CI testing for the JS code
Added documentation on running the tests

chrisjsewell · 2020-06-12T23:37:33Z

Note to self, you can also use https://github.com/jsdom/jsdom to mock the DOM

chrisjsewell · 2020-06-13T02:41:19Z

Ok I've gone ahead and implemented the regexp matching, see: https://143-140029949-gh.circle-artifacts.com/0/html/index.html#using-regexp-prompt-identifiers

The one thing that I won't add in this PR, but should be considered for completeness is regexes per selector, e.g. if you have

$ cd path

and

>>> print("$ cd path")
$ cd path

You only want the first one to copy the $ line

championshuttler · 2020-06-13T19:08:55Z

@chrisjsewell , just want to make sure, is it going to help in excluding $ while copying the commands? Thanks

chrisjsewell · 2020-06-13T20:05:02Z

is it going to help in excluding $ while copying the commands

Well give it a go, and see if it works as expected 😬 https://143-140029949-gh.circle-artifacts.com/0/html/index.html#documentation-builds

championshuttler · 2020-06-13T20:42:41Z

is it going to help in excluding $ while copying the commands

Well give it a go, and see if it works as expected 😬 https://143-140029949-gh.circle-artifacts.com/0/html/index.html#documentation-builds

We are already using it in firefox docs https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html, thank you so much for this extension, its just new contributors copy paste as it is then they faced the problem, so it would be really helpful for a lot of people if somehow we can prevent $ to be copied

chrisjsewell · 2020-06-13T20:56:03Z

We are already using it in firefox docs

Oh thats cool, but I think you misunderstand; the link I gave links to the documentation created using this PR, which (I believe) showcases exactly what you are after

championshuttler · 2020-06-13T21:15:25Z

We are already using it in firefox docs

Oh thats cool, but I think you misunderstand; the link I gave links to the documentation created using this PR, which (I believe) showcases exactly what you are after

oops my bad, just clicked on copy button, yeah it worked perfectly, thank you so much for doing it :)

choldgraf

I really like this - infrastructure is much-improved, and the feature seems great to me. See one comment about using two lists for configuration rather than an "is_regex" flag - what do you think about that? Other than that, I think this is looking close to ready for merge

doc/conf.py

choldgraf · 2020-06-15T18:39:32Z

doc/conf.py

@@ -98,7  98,8 @@
 # html_sidebars = {}

 # CopyButton configuration
-copybutton_prompt_text = ">>> "
+copybutton_prompt_text = ">>> |\\\\$ |\\[\\d*\\]: |\\.\\.\\.: "


One thought - could we simplify this a bit for people by allowing for a list instead of a single regexp? We could even allow for two configs like:

copybutton_prompt_start_strings = [">>>", "..."] copybutton_prompt_start_regexes = ["\\[\\d*\\]"]

I suggest this because I think these should probably be the "defaults" anyway, and then if
people wanted to add a new "starting prompt" they can do so without needing to use regexes

Hmm, I can see some merit, but I guess the main thing I don't like is that it breaks back compatibility: I like that the current solution uses the original copybutton_prompt_text, then just adds a flag to turn on regexes.
Also it may not be as a compatible with what I think could be a next step, to improve robustness (as mentioned in #82 (comment)); to have a mapping of selectors to prompts, e.g something like.

copybutton_prompt_text = { "div higlight-console": "\\\\$ ", "div higlight-python": ">>> ", "div highlight-ipython": "\\[\\d*\\]: |\\.\\.\\.: " }

hmm good point about backwards compatibility - we could keep the same original configuration and just add the regex one on top of it, and change behavior so that both will accept either a string (in which case it'll become a one-string list) or a list of strings?

for the dictionary extension, I agree that'd give us more precision. Do you think people will run into clashes between languages and starting characters?

we could keep the same original configuration and just add the regex one on top of it, and change behavior so that both will accept either a string (in which case it'll become a one-string list) or a list of strings?

This feels more complicated than just using regexes lol? Especially given its only something they have to setup once, then never need to think about again.

Do you think people will run into clashes between languages and starting characters?

Well its probably not massively likely; really depends on people's own documentation. But I personally would rather take the time to add this in my/our repos, just to be sure

doc/conf.py

Co-authored-by: Chris Holdgraf <[email protected]>

chrisjsewell · 2020-06-15T22:18:09Z

I really like this

thanks 😁

choldgraf · 2020-06-17T18:18:48Z

alright lets merge this in and we can decide if we want to add a "language: regexp" mapping later on.

I think one thing we can do is add a documentation section for "common regexes" since most people are not familiar with regex.

Thanks @chrisjsewell for the improvement, much better and thanks for going the extra mile and giving the testing infra the Chris S. treatment :-)

chrisjsewell · 2020-06-17T18:21:37Z

alright lets merge this in and we can decide if we want to add a "language: regexp" mapping later on.

You read my mind 😄

and thanks for going the extra mile and giving the testing infra the Chris S. treatment

haha cheer, shame some certain others don't share my love of tests 👀 😆

championshuttler · 2020-06-23T12:38:04Z

@chrisjsewell thanks for doing this i have a doubt do we have to add this https://github.com/executablebooks/sphinx-copybutton/blob/master/doc/conf.py#L101-L102 in our configuration file to prevent copying $

~~thanks~~

Edit - Got it thanks for it

chrisjsewell added 6 commits June 12, 2020 22:00

Move format function to seperate file

1b5d3ea

inject format func with jinja

58e277e

Update integration.yml

9653cb7

Update copybutton_funcs.js

b1142bf

Add javascript tests

4e7b5ee

Add documentation on testing

91199d9

chrisjsewell requested a review from choldgraf June 12, 2020 23:21

fix code blocks

3fc7c6e

chrisjsewell added 4 commits June 13, 2020 00:52

Update integration.yml

b206f0a

parameterise js tests

ffaffa5

simplify code

a7e53bc

Add regexp matching

556e499

chrisjsewell changed the title ~~Improve package code style and testing~~ Add regexp prompt matching ( code style and testing) Jun 13, 2020

chrisjsewell changed the title ~~Add regexp prompt matching ( code style and testing)~~ Add regexp prompt matching ( style and testing) Jun 13, 2020

chrisjsewell mentioned this pull request Jun 13, 2020

feature request: configurability of copyability #52

Closed

choldgraf reviewed Jun 15, 2020

View reviewed changes

Apply suggestions from code review

d9cd423

Co-authored-by: Chris Holdgraf <[email protected]>

choldgraf merged commit 468be5c into master Jun 17, 2020

choldgraf deleted the prefix-regexes branch June 17, 2020 18:18

chrisjsewell mentioned this pull request Jun 17, 2020

bump version for release #83

Merged

chrisjsewell mentioned this pull request Jun 17, 2020

Docs: Update sphinx-copybutton dep aiidateam/aiida-core#4182

Merged

chrisjsewell mentioned this pull request Jul 8, 2020

👌 IMPROVE: Use Jupyter notebooks for executing "Running calculations" section aiidateam/aiida-tutorials#247

Open

chrisjsewell mentioned this pull request Jul 22, 2020

Javascript implementation of MyST markdown executablebooks/meta#70

Closed

This was referenced Oct 5, 2021

Add sphinx-copybutton to documentation poliastro/poliastro#1324

Closed

docs: Use sphinx-copybutton prompt regex to fully capture examples scikit-hep/pyhf#1617

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add regexp prompt matching ( style and testing) #82

Add regexp prompt matching ( style and testing) #82

chrisjsewell commented Jun 12, 2020 •

edited

Loading

chrisjsewell commented Jun 12, 2020

chrisjsewell commented Jun 13, 2020

championshuttler commented Jun 13, 2020 •

edited

Loading

chrisjsewell commented Jun 13, 2020

championshuttler commented Jun 13, 2020

chrisjsewell commented Jun 13, 2020

championshuttler commented Jun 13, 2020

choldgraf left a comment

choldgraf Jun 15, 2020

chrisjsewell Jun 15, 2020

choldgraf Jun 15, 2020

chrisjsewell Jun 17, 2020

chrisjsewell commented Jun 15, 2020

choldgraf commented Jun 17, 2020 •

edited

Loading

chrisjsewell commented Jun 17, 2020

championshuttler commented Jun 23, 2020 •

edited

Loading

Add regexp prompt matching ( style and testing) #82

Add regexp prompt matching ( style and testing) #82

Conversation

chrisjsewell commented Jun 12, 2020 • edited Loading

chrisjsewell commented Jun 12, 2020

chrisjsewell commented Jun 13, 2020

championshuttler commented Jun 13, 2020 • edited Loading

chrisjsewell commented Jun 13, 2020

championshuttler commented Jun 13, 2020

chrisjsewell commented Jun 13, 2020

championshuttler commented Jun 13, 2020

choldgraf left a comment

Choose a reason for hiding this comment

choldgraf Jun 15, 2020

Choose a reason for hiding this comment

chrisjsewell Jun 15, 2020

Choose a reason for hiding this comment

choldgraf Jun 15, 2020

Choose a reason for hiding this comment

chrisjsewell Jun 17, 2020

Choose a reason for hiding this comment

chrisjsewell commented Jun 15, 2020

choldgraf commented Jun 17, 2020 • edited Loading

chrisjsewell commented Jun 17, 2020

championshuttler commented Jun 23, 2020 • edited Loading

chrisjsewell commented Jun 12, 2020 •

edited

Loading

championshuttler commented Jun 13, 2020 •

edited

Loading

choldgraf commented Jun 17, 2020 •

edited

Loading

championshuttler commented Jun 23, 2020 •

edited

Loading