Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

usability and cross browser compat for completer #1082

Closed
wants to merge 4 commits into from

3 participants

@Carreau
Owner

Improve notebook completer according to @stefanv feedback

  • intercept < backspace > to avoid ff going to previous page when in completer which is really annoying...
  • dissmiss the completer, append what alredy type Plus one caracter in some cases

List of special caracter that are handle are in a given list ()[]./\-+
for the moment. usefull for exaple when typing :

>>> np.s<tab>in(

and not having to type '(' twice.

they are handle separately has the [a-zA-Z] ones because otherwise they
will screw up the regexp, and are opt-in to avoid bugs with invisible
caracters send because some browser have 'keypress' event for meta keys

close #1080

Note to this commit

list of test for the completer across browser with --pylab=inline flag

#test direct one completion
plt.an<tab>       -> plt.annotate

#test filter,tab, only one completion
plt.a<tab>n<tab>  -> plt.annotate

# test partial common beggining
# test dismmised if user erase
plt.a<tab>nn<backspace><backspace>u<tab>                -> completer to `aut`
........................................<tab><tab><tab> -> nothing should append
.......................................................<backspace><backspace<backspace> -> completer cancelled

#test dismiss if no more completion
plt.s<tab>c  -> completer 3 choices
...........u -> dismissed whith what user have typed.  `plt.scu`

# test dismiss in no completion, special symbol
# opt-in list of caracters +-/\()[].
np<tab>.s          -> a 'dot' sould dismiss the completer and be appended
.........<tab>in(  -> np.sin(
np.s<tab>in[       -> np.sin[

I'm a little concerned about keyboard mapping also... but at least i'm using it on french keyboard and most of you should be using english one, so if it's incompatible, I'll now it soon...

@Carreau
Owner

hum...preventing default on <backspace>seem to prevent Firefox from going one previous page, but block deletion in codemirror... I don't know what to do...

@Carreau Carreau closed this
@Carreau Carreau usability and cross browser compat for completer
	- dissmiss the completer, append what alredy type, **Plus** one caracter in
	  some cases

	  List of special caracter that are handle are in a given list `()[]./\-+`
	  for the moment. usefull for exaple when typing :
	  >>> np.s<tab>in(
	  and not having to type '(' twice.

	  they are handle separately has the [a-zA-Z] ones because otherwise they
	  will screw up the regexp, and are opt-in to avoid bugs with invisible
	  caracters send because some browser have 'keypress' event for meta keys

	  close #1080

		Note to this commit :
	  list of test for the completer across browser with --pylab=inline flag

	  #test direct one completion
	  plt.an<tab>       -> plt.annotate

	  #test filter,tab, only one completion
	  plt.a<tab>n<tab>  -> plt.annotate

	  # test partial common beggining
	  # test dismmised if user erase
	  plt.a<tab>nn<backspace><backspace>u<tab>                -> completer to `aut`
	  ........................................<tab><tab><tab> -> nothing should append
	  .......................................................<backspace><backspace<backspace> -> completer cancelled

	  #test dismiss if no more completion
	  plt.s<tab>c  -> completer 3 choices
	  ...........u -> dismissed whith what user have typed.  `plt.scu`

	  # test dismiss in no completion, special symbol
	  # opt-in list of caracters +-/\()[].
	  np<tab>.s          -> a 'dot' sould dismiss the completer and be appended
	  .........<tab>in(  -> np.sin(
	  np.s<tab>in[       -> np.sin[
6c78e9b
@Carreau
Owner

Totally unable to reproduce the bug that had me preventing default with backspace... I'll put that on a browser cache not flushed...
So I removed the code ammended, force push and reopen pull request...
new head is 6c78e9b but gihub seem to not see it...

@Carreau Carreau reopened this
@minrk
Owner

@Carreau - it could be that GitHub stops tracking a branch after closing a PR, so this one won't update. You might close this / reopen as a new one.

@fperez
Owner

Yup, @Carreau, just open a new one and reference this so we can find the earlier discussion.

Small note: don't indent the text in your original message unless you mean it to have code-like formatting (see above how it looks).

@Carreau
Owner

Github saw the changes while as was commuting apparently.
the indented text is the commit message... won't indent anymore...

IPython/frontend/html/notebook/static/js/codecell.js
((6 lines not shown))
isCompSymbol : function (code)
- {return ((code>64 && code <=122)|| code == 189)}
+ {
@fperez Owner
fperez added a note

No need for an extra blank line here, you can put the first { return... here.

@fperez Owner
fperez added a note

Ah, never mind: I see this is a function definition, not inline code... Ignore my comment above.

Just getting used to reading JS :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
IPython/frontend/html/notebook/static/js/codecell.js
@@ -407,6 +418,12 @@ var IPython = (function (IPython) {
// nothing on Shift
return;
}
+ if (key.dismissAndAppend(code) && press) {
+ var newchar = String.fromCharCode(code);
+ typed_characters=typed_characters+newchar;
@fperez Owner
fperez added a note

whitespace around =

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Carreau
Owner

@fperez,
white spaced, getting used to write js.

@fperez
Owner

It mostly looks good, but I'm still getting a little usability nag: try, with --pylab

  1. type plt.pl
  2. hit <tab>
  3. All the completions start with 'plot', but the 'ot' aren't immediately written to the page. They are, however, recorded internally. So if I add the letter 'o', thinking that it needs one more letter, I get the completer to write out plt.ploto, which doesn't make any sense.

Something still needs fixing there...

@fperez
Owner

Sorry, in the previous one it was plt.pl, not np.pl. My mistake, corrected at github but you'll get the wrong version by email.

@Carreau
Owner

Thats the behaviour of the as you type completer since the beginnig and the use of the 'blue field' on top of the completer is to know the internal state.
My fisrt thought was to do as you suggested, that is to say append each keypress into the codemirror field, but then how would you know when the completer was invoked ? It is especially anoying when you start to correct what you typed and have the completer dismissed because you over <backspace>'d.

IMHO, you should absolutely have at least one indication of what was typed because the filtering is based on the result send by the kernel when pressing <tab> for the first time.

If you don't agree I can try to better entangle it with code mirror. And if you get a better idea of visual indication i'll take it.

@fperez
Owner

Idea: in the blue field, show the characters that start the completion with a different background color, say gray, and only newly typed characters have a blue background. That gives you a good visual cue of the completion state. But do append every typed character onto the underlying codemirror text area, so that the above confusion doesn't happen. With the gray/blue boundary, at least you know if you're about to backspace over your initial input and get the completer dismissed.

@Carreau
Owner

That's seem a good idea. I'll see what I can do.

@Carreau Carreau completer update code-miror on the fly
	Following @fperez advice, change the completer apparence to avoid user confusion.

	- Append what the user type in the completer in code-miror, (Almost) as if
	  codemirror still have focus
	- distinguish between "fixed" completion  part, which was sent to the kernel
	  (now written in bold) and filtering one,handled only in JS,that the user
	  can errase without dismissing the completer

	I changed the action of <Space> to dismiss the completer with what have
	already been typed and inserting a space instead of "picking" the currently
	hilighted option

	<Escape> will still dissmiss the completer and remove everything the user as
	typed since the completer invocation

	Note that while the completer is shown, code-mirror does not show any
	blinking cursor
f22fcea
@Carreau
Owner

@fperez, It more User friendly like that ?
( Note that I change the behaviour of Space from "pick" to "dismiss and append" )

@fperez
Owner

@Carreau, something happened but now on Chrome this completely breaks the completion. Weird...

Try plt.ac<TAB>. The only completion should be plt.acorr, but nothing happens. So with this PR, if you hit 'enter' on the completions list, it works, but the actual TAB key isn't working anymore...

@Carreau
Owner

actually, the notebook is not working anymore today, even on master or before this branch
I can't even execute a cell... ( silent chrome update ?) when I'll get it working (again), i'll take a look

@fperez
Owner

MMh, that's strange. It works perfectly fine for me, both on Chrome and on Firefox...

@Carreau
Owner

Closing, and continuing on rebased ( #1108 ) with the fix

@Carreau Carreau closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 1, 2011
  1. @Carreau

    usability and cross browser compat for completer

    Carreau authored
    	- dissmiss the completer, append what alredy type, **Plus** one caracter in
    	  some cases
    
    	  List of special caracter that are handle are in a given list `()[]./\-+`
    	  for the moment. usefull for exaple when typing :
    	  >>> np.s<tab>in(
    	  and not having to type '(' twice.
    
    	  they are handle separately has the [a-zA-Z] ones because otherwise they
    	  will screw up the regexp, and are opt-in to avoid bugs with invisible
    	  caracters send because some browser have 'keypress' event for meta keys
    
    	  close #1080
    
    		Note to this commit :
    	  list of test for the completer across browser with --pylab=inline flag
    
    	  #test direct one completion
    	  plt.an<tab>       -> plt.annotate
    
    	  #test filter,tab, only one completion
    	  plt.a<tab>n<tab>  -> plt.annotate
    
    	  # test partial common beggining
    	  # test dismmised if user erase
    	  plt.a<tab>nn<backspace><backspace>u<tab>                -> completer to `aut`
    	  ........................................<tab><tab><tab> -> nothing should append
    	  .......................................................<backspace><backspace<backspace> -> completer cancelled
    
    	  #test dismiss if no more completion
    	  plt.s<tab>c  -> completer 3 choices
    	  ...........u -> dismissed whith what user have typed.  `plt.scu`
    
    	  # test dismiss in no completion, special symbol
    	  # opt-in list of caracters +-/\()[].
    	  np<tab>.s          -> a 'dot' sould dismiss the completer and be appended
    	  .........<tab>in(  -> np.sin(
    	  np.s<tab>in[       -> np.sin[
  2. @Carreau

    Apply pep8 to js

    Carreau authored
Commits on Dec 3, 2011
  1. @Carreau

    completer update code-miror on the fly

    Carreau authored
    	Following @fperez advice, change the completer apparence to avoid user confusion.
    
    	- Append what the user type in the completer in code-miror, (Almost) as if
    	  codemirror still have focus
    	- distinguish between "fixed" completion  part, which was sent to the kernel
    	  (now written in bold) and filtering one,handled only in JS,that the user
    	  can errase without dismissing the completer
    
    	I changed the action of <Space> to dismiss the completer with what have
    	already been typed and inserting a space instead of "picking" the currently
    	hilighted option
    
    	<Escape> will still dissmiss the completer and remove everything the user as
    	typed since the completer invocation
    
    	Note that while the completer is shown, code-mirror does not show any
    	blinking cursor
Commits on Dec 6, 2011
  1. @Carreau
This page is out of date. Refresh to see the latest.
View
5 IPython/frontend/html/notebook/static/css/notebook.css
@@ -406,6 +406,11 @@ div.text_cell_render {
min-height:50px;
}
+/*fixed part of the completion*/
+.completions p b{
+ font-weight:bold;
+}
+
.completions p{
background: #DDF;
/*outline: none;
View
127 IPython/frontend/html/notebook/static/js/codecell.js
@@ -216,7 +216,7 @@ var IPython = (function (IPython) {
tooltip.append(expandlink);
tooltip.append(morelink);
if(defstring){
- defstring_html= $('<pre/>').html(utils.fixConsole(defstring));
+ defstring_html = $('<pre/>').html(utils.fixConsole(defstring));
tooltip.append(defstring_html);
}
tooltip.append(pre);
@@ -239,12 +239,23 @@ var IPython = (function (IPython) {
var key = { tab:9,
esc:27,
backspace:8,
- space:13,
+ space:32,
shift:16,
- enter:32,
- // _ is 189
+ enter:13,
+ // _ is 95
isCompSymbol : function (code)
- {return ((code>64 && code <=122)|| code == 189)}
+ {
+ return (code > 64 && code <= 90)
+ || (code >= 97 && code <= 122)
+ || (code == 95)
+ },
+ dismissAndAppend : function (code)
+ {
+ chararr = ['(',')','[',']','+','-','/','\\','.',' '];
+ codearr = chararr.map(function(x){return x.charCodeAt(0)});
+ return jQuery.inArray(code, codearr) != -1;
+ }
+
}
// smart completion, sort kwarg ending with '='
@@ -253,25 +264,26 @@ var IPython = (function (IPython) {
{
kwargs = new Array();
other = new Array();
- for(var i=0;i<matches.length; ++i){
+ for(var i = 0 ; i<matches.length ; ++i){
if(matches[i].substr(-1) === '='){
kwargs.push(matches[i]);
}else{other.push(matches[i]);}
}
newm = kwargs.concat(other);
- matches=newm;
+ matches = newm;
}
// end sort kwargs
// give common prefix of a array of string
function sharedStart(A){
+ if(A.length == 1){return A[0]}
if(A.length > 1 ){
- var tem1, tem2, s, A= A.slice(0).sort();
- tem1= A[0];
- s= tem1.length;
- tem2= A.pop();
- while(s && tem2.indexOf(tem1)== -1){
- tem1= tem1.substring(0, --s);
+ var tem1, tem2, s, A = A.slice(0).sort();
+ tem1 = A[0];
+ s = tem1.length;
+ tem2 = A.pop();
+ while(s && tem2.indexOf(tem1) == -1){
+ tem1 = tem1.substring(0, --s);
}
return tem1;
}
@@ -281,8 +293,8 @@ var IPython = (function (IPython) {
//try to check if the user is typing tab at least twice after a word
// and completion is "done"
- fallback_on_tooltip_after=2
- if(matches.length==1 && matched_text === matches[0])
+ fallback_on_tooltip_after = 2
+ if(matches.length == 1 && matched_text === matches[0])
{
if(this.npressed >fallback_on_tooltip_after && this.prevmatch==matched_text)
{
@@ -292,13 +304,13 @@ var IPython = (function (IPython) {
this.request_tooltip_after_time(matched_text+'(',0,this);
return;
}
- this.prevmatch=matched_text
- this.npressed=this.npressed+1;
+ this.prevmatch = matched_text
+ this.npressed = this.npressed+1;
}
else
{
- this.prevmatch="";
- this.npressed=0;
+ this.prevmatch = "";
+ this.npressed = 0;
}
// end fallback on tooltip
//==================================
@@ -311,23 +323,29 @@ var IPython = (function (IPython) {
var close = function () {
if (done) return;
done = true;
- if (complete!=undefined)
+ if (complete != undefined)
{complete.remove();}
that.is_completing = false;
that.completion_cursor = null;
};
- // insert the given text and exit the completer
- var insert = function (selected_text, event) {
+ // update codemirror with the typed text
+ prev = matched_text
+ var update = function (inserted_text, event) {
that.code_mirror.replaceRange(
- selected_text,
+ inserted_text,
{line: cur.line, ch: (cur.ch-matched_text.length)},
- {line: cur.line, ch: cur.ch}
+ {line: cur.line, ch: (cur.ch+prev.length-matched_text.length)}
);
+ prev = inserted_text
if(event != null){
event.stopPropagation();
event.preventDefault();
}
+ };
+ // insert the given text and exit the completer
+ var insert = function (selected_text, event) {
+ update(selected_text)
close();
setTimeout(function(){that.code_mirror.focus();}, 50);
};
@@ -348,22 +366,23 @@ var IPython = (function (IPython) {
// Used to 'pick' when pressing tab
if (matches.length < 1) {
insert(typed_text,event);
- if(event !=null){
+ if(event != null){
event.stopPropagation();
event.preventDefault();
}
- } else if (autopick && matches.length==1) {
+ } else if (autopick && matches.length == 1) {
insert(matches[0],event);
- if(event !=null){
+ if(event != null){
event.stopPropagation();
event.preventDefault();
}
}
//clear the previous completion if any
+ update(typed_text,event);
complete.children().children().remove();
- $('#asyoutype').text(typed_text);
- select=$('#asyoutypeselect');
- for (var i=0; i<matches.length; ++i) {
+ $('#asyoutype').html("<b>"+matched_text+"</b>"+typed_text.substr(matched_text.length));
+ select = $('#asyoutypeselect');
+ for (var i = 0; i<matches.length; ++i) {
select.append($('<option/>').html(matches[i]));
}
select.children().first().attr('selected','true');
@@ -372,7 +391,7 @@ var IPython = (function (IPython) {
// create html for completer
var complete = $('<div/>').addClass('completions');
complete.attr('id','complete');
- complete.append($('<p/>').attr('id', 'asyoutype').html(matched_text));//pseudo input field
+ complete.append($('<p/>').attr('id', 'asyoutype').html('<b>fixed part</b>user part'));//pseudo input field
var select = $('<select/>').attr('multiple','true');
select.attr('id', 'asyoutypeselect')
@@ -390,25 +409,31 @@ var IPython = (function (IPython) {
// So a first actual completion. see if all the completion start wit
// the same letter and complete if necessary
fastForward = sharedStart(matches)
- typed_characters= fastForward.substr(matched_text.length);
+ typed_characters = fastForward.substr(matched_text.length);
complete_with(matches,matched_text+typed_characters,true,null);
- filterd=matches;
+ filterd = matches;
// Give focus to select, and make it filter the match as the user type
// by filtering the previous matches. Called by .keypress and .keydown
var downandpress = function (event,press_or_down) {
var code = event.which;
var autopick = false; // auto 'pick' if only one match
if (press_or_down === 0){
- press=true; down=false; //Are we called from keypress or keydown
+ press = true; down = false; //Are we called from keypress or keydown
} else if (press_or_down == 1){
- press=false; down=true;
+ press = false; down = true;
}
if (code === key.shift) {
// nothing on Shift
return;
}
- if (code === key.space || code === key.enter) {
- // Pressing SPACE or ENTER will cause a pick
+ if (key.dismissAndAppend(code) && press) {
+ var newchar = String.fromCharCode(code);
+ typed_characters = typed_characters+newchar;
+ insert(matched_text+typed_characters,event);
+ return
+ }
+ if (code === key.enter) {
+ // Pressing ENTER will cause a pick
event.stopPropagation();
event.preventDefault();
pick();
@@ -416,35 +441,41 @@ var IPython = (function (IPython) {
// We don't want the document keydown handler to handle UP/DOWN,
// but we want the default action.
event.stopPropagation();
- //} else if ( key.isCompSymbol(code)|| (code==key.backspace)||(code==key.tab && down)){
- } else if ( (code==key.backspace)||(code==key.tab && down) || press || key.isCompSymbol(code)){
+ } else if ( (code == key.backspace)||(code == key.tab && down) || press || key.isCompSymbol(code)){
if( key.isCompSymbol(code) && press)
{
var newchar = String.fromCharCode(code);
- typed_characters=typed_characters+newchar;
+ typed_characters = typed_characters+newchar;
} else if (code == key.tab) {
fastForward = sharedStart(filterd)
ffsub = fastForward.substr(matched_text.length+typed_characters.length);
- typed_characters=typed_characters+ffsub;
- autopick=true;
- event.stopPropagation();
- event.preventDefault();
+ typed_characters = typed_characters+ffsub;
+ autopick = true;
} else if (code == key.backspace && down) {
// cancel if user have erase everything, otherwise decrease
// what we filter with
+ event.preventDefault();
if (typed_characters.length <= 0)
{
insert(matched_text,event)
+ return
}
- typed_characters=typed_characters.substr(0,typed_characters.length-1);
- }else{return}
+ typed_characters = typed_characters.substr(0,typed_characters.length-1);
+ } else if (press && code != key.backspace && code != key.tab && code != 0){
+ insert(matched_text+typed_characters,event);
+ return
+ } else {
+ return
+ }
re = new RegExp("^"+"\%?"+matched_text+typed_characters,"");
filterd = matches.filter(function(x){return re.test(x)});
complete_with(filterd,matched_text+typed_characters,autopick,event);
- } else if(down){ // abort only on .keydown
+ } else if( code == key.esc) {
+ // dismiss the completer and go back to before invoking it
+ insert(matched_text,event);
+ } else if( press ){ // abort only on .keypress or esc
// abort with what the user have pressed until now
console.log('aborting with keycode : '+code+' is down :'+down);
- insert(matched_text+typed_characters,event);
}
}
select.keydown(function (event) {
Something went wrong with that request. Please try again.