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

Keypress event (conditional) trigger #81

Merged
merged 5 commits into from
Nov 8, 2015

Conversation

arichiardi
Copy link
Contributor

This is an implementation of the key press trigger.

It is conditional because if you push a function(current_text, new_char) with SetKeyPressPredicate and your function returns false (default), no event is triggered. This is done cause you might want to add logic, say, if the last word of the current text + new_char is a keyword then trigger, you can achieve it here.

It works quite nicely here 👍

Please let me know if I have been too prolific with the function names 😄

@@ -694,6 +698,7 @@ class JQConsole
# (e.g. ">>> "), and the editable text to the left and right of the cursor.
@$prompt_label = $(EMPTY_SPAN).appendTo @$prompt_current
@$prompt_left = $(EMPTY_SPAN).appendTo @$prompt_current
@$prompt_left.attr 'class', CLASS_USER_TEXT
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is for identifying the part of the prompt that is changed by the user. This was useful for implementing highlighting (with highlight.js) but maybe there are other way that I am not considering (not js expert here 😄)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh we use GetPromptText great!

@amasad
Copy link
Contributor

amasad commented Nov 7, 2015

Looks good. Can you add an example usage? Would be great to add the documentation too.

@arichiardi
Copy link
Contributor Author

Where do you want me to add the usage? In the coffee or in the README ?

@amasad
Copy link
Contributor

amasad commented Nov 8, 2015

Read me and an example usage would be great!

On Saturday, November 7, 2015, Andrea Richiardi notifications@github.com
wrote:

Where do you want me to add the usage? In the coffee or in the README ?


Reply to this email directly or view it on GitHub
#81 (comment).

@arichiardi arichiardi force-pushed the keypress-event branch 3 times, most recently from 2c9adfa to b0e4b81 Compare November 8, 2015 02:38
@arichiardi
Copy link
Contributor Author

Ok done, have a look in index.html and tell me if it's good 😄

@amasad
Copy link
Contributor

amasad commented Nov 8, 2015

That's very cool. Thank you. Will do a release soon after I fix #80

amasad added a commit that referenced this pull request Nov 8, 2015
Keypress event (conditional) trigger
@amasad amasad merged commit 869f1f2 into replit-archive:master Nov 8, 2015
@arichiardi
Copy link
Contributor Author

👍 thanks, as before I would like to push a release in cljsjs too. I am relying on jqconsole for an upcoming app and a fast release would really make my job more comfortable. Thanks for the library if I haven't already told you 😄

@arichiardi arichiardi deleted the keypress-event branch November 8, 2015 02:55
@amasad
Copy link
Contributor

amasad commented Nov 8, 2015

I'd like to simplify this a little bit. Instead of having two different handlers, let's make it a single step. The user gets to execute the handler before the internal handler with no auxiliary events needed. So the example would be.

jqconsole.SetKeyPressHandler(function (e) {
  // do stuff
  // the internal handler should not execute.
  return false;
});

@amasad
Copy link
Contributor

amasad commented Nov 8, 2015

I'll send a PR and let me know what you think

@arichiardi
Copy link
Contributor Author

Ok, I put two because one gives the "pace" of the triggering (I don't won't to trigger every keypress for instance), the other handles.
In any case, I will wait for your PR :)

amasad added a commit that referenced this pull request Nov 8, 2015
Based on @arichiardi's work in #81. This introduces a way to set
custom key handlers:

* key press handler: concerned with text input
* control key handler: concerned with other key handler

The handlers can return false to prevent jqconsole from handling the
event.
@amasad
Copy link
Contributor

amasad commented Nov 8, 2015

see #82 gives us the full potential for auto-complete and other user-defined behavior

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.

2 participants