Fix regex escaping #88

s-weigand · 2020-07-21T22:32:11Z

As concluded in #86, this fixes the way that copybutton_prompt_text is injected into copybutton.js_t.
And updates the docs on how to use the regex feature.
I left the old tests in there, since I kind like those oddities if you know them. But if that is too much clutter for you I can remove them.

Not sure how much you value SemVer but this is a breaking change, since it will break functionality for people that already use the regex feature.

fixes #86

Note: I left #84 (besides) untouched, since IMHO this should configurable i.e. by a variable copy_blank_lines. I just can see someone complaining around the corner, even so 'copy as is' is the most natural thing and should be default 😒

this uses raw string formatting an prevents the loss of too many backslashes and also updated the configuration for the documentation

choldgraf · 2020-07-22T22:54:27Z

Thanks for this patch! @chrisjsewell do you think you could double-check that this looks correct? My regex skills are very poor 😬

chrisjsewell

Yep mainly good thanks, I just think we can probably remove the "old style"

chrisjsewell · 2020-07-22T23:20:53Z

doc/index.rst


 .. code-block:: restructuredtext

+   Old style:


Do we actually need this old and new style? Was old style actually ever a thing (i.e. was it used in old versions of ipython) or can we just remove it (the examples and in the regex) and keep only the "new" style

I also wasn't sure if the "old style" was ever like this, ipython console, jupyter console and qt-console use the "new style".
But I thought "there had to be a reason why [1] was used" (that's why I came up with the "old" and "new"). So If you can't think of an "old" use case, I will remove it.

Also, whlie derping around with ipython, jupyter console and jupyter qtconsole I found differences with line continuation:

ipython and jupyter qtconsole:

In [1]: def test(): ...: return 1

jupyter console:

In [1]: def test(): : return 1

So the regexes should be r"\s \.\.\.: " and r"\s : "

What do you think about replacing:

"Old style" with "ipython and qtconsole style"

"New style" with "jupyter console style"

Also using the regexes r" {2,5}\.\.\.: " and r" {5,8}: " would support continuations for In []: up to In [999], an be more robust against false positive matches than my proposed version with \s .

didn't even know there was a jupyter console lol, but yeh sounds good to me if you want to have the two examples, or just use ipython

sphinx_copybutton/_static/test.js

chrisjsewell · 2020-07-22T23:26:41Z

Not sure how much you value SemVer but this is a breaking change, since it will break functionality for people that already use the regex feature.

Yep we should release this as 0.3.0

s-weigand · 2020-07-22T23:57:48Z

Not sure how much you value SemVer but this is a breaking change, since it will break functionality for people that already use the regex feature.

Yep we should release this as 0.3.0

Well, as for SemVer that would be a 1. 0. 0 as it might break things.IMHO SemVer only makes sense if it is consistent, which manually is hard to do and I didn't find the proper tool for it in the python universe / GH actions to automate it for me (maybe you can give me a pointer 😉).
I.e. in a project we had a dependency which changed from 0.9.9 to 1.0.0 and all was fine.
But when they went to 1.0.1 many tests went down, so IMHO inconsistent versioning is more harmfull.
I think the easiest versioning scheme to maintain, is the date one as pip uses if for about 1.5y.

chrisjsewell · 2020-07-23T00:06:06Z

Well, as for SemVer that would be a 1. 0. 0

Yeh it's a bit of a cheat, but for versioning < 1.0.0, I generally treat MINOR version changes as also being backwards-incompatible, and PATCH for anything else. I think that's reasonable, since below 1.0 you're generally in "beta" mode where users should not yet expect the code to be completely "stable" anyway

choldgraf · 2020-07-23T00:12:48Z

Yep I'm in the same boat - IMO pre 1.0 you treat "minor" versions as "major" versions, and pre 0.1 you treat "patch" versions as "major" versions

s-weigand · 2020-07-23T00:13:55Z

Well, as for SemVer that would be a 1. 0. 0

Yeh it's a bit of a cheat, but for versioning < 1.0.0, I generally treat MINOR version changes as also being backwards-incompatible, and PATCH for anything else. I think that's reasonable, since below 1.0 you're generally in "beta" mode where users should not yet expect the code to be completely "stable" anyway

Yeah, fair enough, versions below 1.0 are a gray area ^^

chrisjsewell · 2020-07-23T00:15:10Z

maybe you can give me a pointer

Well I don't know it there can be a completely programmatic identifier of back-compatability breaking.
But I'm trying to push this commit convention now https://executablebooks.org/en/latest/dev-conventions.html#commit-messages, which hopefully will help to make it clearer where breaking changes occur,
i.e. the commit from this PR will start: ‼️ BREAKING:

s-weigand · 2020-07-23T00:33:25Z

maybe you can give me a pointer

Well I don't know it there can be a completely programmatic identifier of back-compatability breaking.
But I'm trying to push this commit convention now https://executablebooks.org/en/latest/dev-conventions.html#commit-messages, which hopefully will help to make it clearer where breaking changes occur,
i.e. the commit from this PR will start: ‼️ BREAKING:

In my TypeScript projects with template typescript-library-starter I use git-cz, which basically wrappes git commits in a CLI dialog and adds keywords which get picked up further down the line. But 'completely programmatic identifier of back-compatability breaking' might be a dream.. enough off topic xD

chrisjsewell · 2020-07-23T00:49:58Z

In my TypeScript projects with template typescript-library-starter I use git-cz

Yeh interesting cheers, there's a few different categories, but quite similar. I guess there's no convention that can cover all eventualities, but its better to have a convention than none at all.

which basically wrappes git commits in a CLI dialog

The only problem with any locally installed "commit tool" is that actually you're mainly writing the commit message directly here in the GitHub PR interface

chrisjsewell · 2020-07-24T20:53:50Z

thanks @s-weigand!

choldgraf · 2020-07-24T21:01:30Z

thanks for the patch @s-weigand !!

see: executablebooks#88 (comment)

s-weigand added 3 commits July 21, 2020 23:30

updated injection of 'copybutton_prompt_text' into 'copybutton.js_t'

d301d79

this uses raw string formatting an prevents the loss of too many backslashes and also updated the configuration for the documentation

Added false positive comment to tests and added tests with proper regexp

8677450

Updated documentation on how to use 'copybutton_prompt_text' with regexp

f33de4f

s-weigand changed the title ~~Regex escaping~~ Fix regex escaping Jul 21, 2020

move false-positive test to end

863efcc

chrisjsewell requested changes Jul 22, 2020

View reviewed changes

chrisjsewell mentioned this pull request Jul 23, 2020

Note about versioning <1.0.0 in developer conventions executablebooks/meta#77

Closed

chrisjsewell mentioned this pull request Jul 23, 2020

Implement Git Commits Convention (w/ Emoji) executablebooks/meta#74

Closed

Renamed old and new to the corresponding consoles and refined regex

93db31d

s-weigand requested a review from chrisjsewell July 24, 2020 19:19

chrisjsewell approved these changes Jul 24, 2020

View reviewed changes

chrisjsewell merged commit 1cd8420 into executablebooks:master Jul 24, 2020

s-weigand deleted the regex_escaping branch July 25, 2020 12:05

s-weigand mentioned this pull request Jul 25, 2020

👌 IMPROVE: Add default python repl continuation regex #91

Merged

s-weigand added a commit to s-weigand/sphinx-copybutton that referenced this pull request Jul 25, 2020

📖 DOC: Added regex explanation and removed 'old' ipython pattern

ff81305

see: executablebooks#88 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix regex escaping #88

Fix regex escaping #88

s-weigand commented Jul 21, 2020

choldgraf commented Jul 22, 2020

chrisjsewell left a comment

chrisjsewell Jul 22, 2020

s-weigand Jul 23, 2020

s-weigand Jul 23, 2020

s-weigand Jul 24, 2020

chrisjsewell Jul 24, 2020

chrisjsewell commented Jul 22, 2020

s-weigand commented Jul 22, 2020

chrisjsewell commented Jul 23, 2020 •

edited

Loading

choldgraf commented Jul 23, 2020

s-weigand commented Jul 23, 2020

chrisjsewell commented Jul 23, 2020

s-weigand commented Jul 23, 2020

chrisjsewell commented Jul 23, 2020

chrisjsewell commented Jul 24, 2020

choldgraf commented Jul 24, 2020

Fix regex escaping #88

Fix regex escaping #88

Conversation

s-weigand commented Jul 21, 2020

choldgraf commented Jul 22, 2020

chrisjsewell left a comment

Choose a reason for hiding this comment

chrisjsewell Jul 22, 2020

Choose a reason for hiding this comment

s-weigand Jul 23, 2020

Choose a reason for hiding this comment

s-weigand Jul 23, 2020

Choose a reason for hiding this comment

s-weigand Jul 24, 2020

Choose a reason for hiding this comment

chrisjsewell Jul 24, 2020

Choose a reason for hiding this comment

chrisjsewell commented Jul 22, 2020

s-weigand commented Jul 22, 2020

chrisjsewell commented Jul 23, 2020 • edited Loading

choldgraf commented Jul 23, 2020

s-weigand commented Jul 23, 2020

chrisjsewell commented Jul 23, 2020

s-weigand commented Jul 23, 2020

chrisjsewell commented Jul 23, 2020

chrisjsewell commented Jul 24, 2020

choldgraf commented Jul 24, 2020

chrisjsewell commented Jul 23, 2020 •

edited

Loading