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

Initial implementation of "see as you type" transaction creation #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jwiegley
Copy link
Member

@enderw88 Here's an initial implementation that shows what I mean. It's not quite ready for prime-time for the following reasons:

  1. Error handling is terrible. It just swallows chance errors so as not to disrupt the display.

  2. It's slow, because it copies over the source buffer for literally every key pressed. But this is to take advantage of "prior knowledge" in what gets displayed, so in a way it's necessary.

  3. It feels fragile overall, and I'm not sure my window management represents best practices.

However, it does work very nicely (for small ledger files at least), showing a colorized live display of what will get inserted.

@jwiegley
Copy link
Member Author

Oh, and it needs to incorporate the "figure out where in the timeline this should go" logic.

@cpitclaudel
Copy link
Contributor

This works really nicely, actually. And it doesn't feel that slow here.
Have you had time to look at it again since you wrote the initial implementation?

@cpitclaudel
Copy link
Contributor

It's slow, because it copies over the source buffer for literally every key pressed. But this is to take advantage of "prior knowledge" in what gets displayed, so in a way it's necessary.

Presumably the rest of the buffer should be frozen while the user builds their transaction, so is it actually necessary to resend the whole buffer every time? Isn't it enough to save a copy at the beginning, and then reuse that?

@cpitclaudel
Copy link
Contributor

Error handling is terrible. It just swallows chance errors so as not to disrupt the display.

I did run into a few issues:

  • Ledger runs assertions from the file while the transaction is being typed, so if ledger infers a transaction that doesn't pass assertions the live window just shows 'assertion failed'

  • It's tricky to pass special characters to ledger, like ' or ;, as they tend to crash ledger-mode. (This is a ledger-mode issue, I think; it's not specific to this patch)

@jwiegley
Copy link
Member Author

@cpitclaudel Great point about only needing the file up to the point of insertion. And no, I haven't looked at this code any further since writing it.

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