[hinterland] make all regexes configurable #815
Conversation
for deciding whether to trigger autocompletion/tooltip
| config[regexp_names[ii]] = new RegExp(config[regexp_names[ii]]); | ||
| } | ||
| catch (err) { | ||
| console.warning(log_prefix, 'error parsing', regexp_names[ii] + ':', err); |
There was a problem hiding this comment.
console.warn() instead of console.warning() for me
There was a problem hiding this comment.
True! Too used to python logging, which seems to accept either
| Compatibility: 4.x | ||
| Parameters: | ||
| - name: enable_at_start |
There was a problem hiding this comment.
hinterland prefix missing for parameters (since the hinterland section is read in the js source)
| if ( pre_cursor !== '' && | ||
| config.include_regexp.test(pre_cursor) && | ||
| !config.exclude_regexp.test(pre_cursor) ) { | ||
| if (config.tooltip_regexp.test(pre_cursor)) { | ||
| cell.tooltip.request(cell); |
There was a problem hiding this comment.
The tooltip_regexp character(s) must be included in the include_regexp characters, otherwise this is not fired. Perhaps that the the two regexp have to be merged (or more simply, the test modified perhaps (config.include_regexp.test(pre_cursor) || config.tooltip_regexp.test(pre_cursor) ) && .. (untested)
There was a problem hiding this comment.
right, good point. Probably simplest/best to alter the test
| value of the notebook's Completer.reinvoke_re parameter is used, which can | ||
| be modified by kernels, but defaults to /[%0-9a-z._/\\:~-]/i | ||
| input_type: text | ||
| default: '' |
There was a problem hiding this comment.
The regex entered here is case sensitive, because the /i flag is not added for now when creating the regex object. Perhaps warn the user on that and provide an example regex, eg [%0-9A-Za-z._/\:~-]
| autocompletion. For python, this is useful for example for function calls, | ||
| so the default regex matches opening parentheses. | ||
| input_type: text | ||
| default: '\\(' |
There was a problem hiding this comment.
It seems that a single '\' is enough: '\(' ; the corresponding re is /\(/ which is correct (and works)
There was a problem hiding this comment.
Actually both work... sorry :)
There was a problem hiding this comment.
right. Depends on the regex engine, some will break given a single unescaped bracket as it would denote the start of a capturing group, but I guess javascript is prepared to treat a single bracket as a literal...
There was a problem hiding this comment.
ah, wait, sorry, I think this is actually to the complexities of YAML string parsing, which shares some similarities with bash: strings in single-quotes are more-or-less like python's 'raw' strings, which I hadn't realised. Those in double quotes allow use of \ to escape special characters. I'm guessing that it still worked only because the yaml default doesn't actually get used, as I'd expect to end up with a regex of /\(/? Anyway, I think I'll switch to use the double-quoted string in the YAML...
|
It works nicely and is very useful. |
|
nice, glad to hear it's useful :) |
for deciding whether to trigger autocompletion/tooltip.
Addresses #651