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

Standardize Buffer text editing interface #20

Closed
ChrisPenner opened this issue Jan 12, 2017 · 9 comments
Closed

Standardize Buffer text editing interface #20

ChrisPenner opened this issue Jan 12, 2017 · 9 comments

Comments

@ChrisPenner
Copy link
Owner

ChrisPenner commented Jan 12, 2017

We shouldn't allow extensions to just edit text however they like because it limits our knowledge of what extensions are doing. If we construct the api such that it uses a series of BufActions which we control; then we maintain flexibility in our implementation in the future.

This involves creating 3 new functions: (https://github.com/ChrisPenner/rasa/blob/master/rasa/src/Rasa/Internal/Editor.hs#L141); e.g. setRange, overRange, getRange; right now the range lens is exposed entirely; but that's going to make it tough to hook into in the future, so we should change it to a set of actions instead.

Here would be the rough steps involved:

  • create 3 new actions:
setRange :: Range -> YiString -> BufAction ()
overRange :: Range -> (YiString -> YiString) -> BufAction ()
getRange :: Range -> BufAction YiString
  • Replace occurrences of the current range lens with the appropriate new actions
  • create a new Event type HERE for BufTextChanged or something like that; I'm thinking we should maybe include the range that was effected and the new text as part of the event? People can ignore it if they like.
  • Inside setRange, overRange add a dispatchEvent for the new BufTextChanged event AFTER editing the buffer text.
  • ... profit?

As for making these events listenable on a 'per-buffer' basis, that's going to largely depend on this issue: #21

@clojj
Copy link
Contributor

clojj commented Jan 12, 2017

dispatchEvent is type Action ()
How to run it inside a BufAction () ... ?

@ChrisPenner
Copy link
Owner Author

ChrisPenner commented Jan 12, 2017

Good catch; That's definitely on the list of things to fix! I'm in the process of revamping the Buffer referencing system right now so I'll put that at the top of my list; ideally all Actions can also run inside a BufAction, but I'm not quite there yet. Should be able to get something working for that this weekend. If you want to proceed, just assume that you can run Actions inside a BufAction, I'll get it working ASAP!

@clojj
Copy link
Contributor

clojj commented Jan 12, 2017

Great.
Converting the tokens to rasa-styles needs thinking in the meantime anyway.

@ChrisPenner
Copy link
Owner Author

Admittedly the rasa-ext-styles interface isn't perfect yet; but I think it makes sense. It allows different extensions to each define styles and merges them together for you, so hopefully you can figure it out :)

It might eventually make sense to express styles generically as PrimaryColor rather than Red so that users can easily define their own color schemes, but that should be a relatively easy change later.

@clojj
Copy link
Contributor

clojj commented Jan 13, 2017

My Lexer runs in another thread.
Is it Ok to dispatchEvent from another thread (started via forkIO) ?
(maybe I should ask this in a separate issue)

@ChrisPenner
Copy link
Owner Author

ChrisPenner commented Jan 13, 2017

Check out doAsync for this; (unfortunately the hackage docs are down for some reason, but there's an example in the docs, see the source)

All Actions must be run synchronously to avoid stepping on each-other's toes; but if you run the Lexer in another thread via doAsync then you can yield an Action as a result which applies the resulting styling. Basically computation can be async (via IO), but altering Editor state cannot.

@clojj
Copy link
Contributor

clojj commented Jan 13, 2017

Thanks.
I guess one problem would be that the tokenized text could have changed, if this runs asynchronously.

So I will first try to tokenize synchronously.

@ChrisPenner
Copy link
Owner Author

Yeah; that's a tricky thing; but of course any async task has to deal with the idea that things may have changed (otherwise it wouldn't be async 😝). Try getting it working synchronously; then we can experiment with different async versions. My personal recommendation would be to run it async; then in the returned Action check if the buffer has changed since we started (via a closure; you can look up the buffer text in the spawning Action and keep a reference in the completion Action); if a change has occurred you can apply the computed style (as a partially correct solution) and then spawn another async action immediately to handle the changes. Does that make any sense?

clojj added a commit to clojj/rasa that referenced this issue Jan 13, 2017
basic setup for synchronous tokenization
clojj added a commit to clojj/rasa that referenced this issue Jan 16, 2017
introduce BufTextChanged event
adresses issue ChrisPenner#20
@clojj clojj mentioned this issue Jan 16, 2017
@ChrisPenner
Copy link
Owner Author

Handled in #28 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants