tarkah / tickrs

Realtime ticker data in your terminal 📈
MIT License
1.16k stars 58 forks source link

add kagi chart #93

Closed tarkah closed 3 years ago

tarkah commented 3 years ago

@miraclx @alfredo-catalano The Kagi chart is basically complete. You can start to test it off this branch if you'd like.

I still need to add ability to define custom reversal amounts from config file and in the GUI.

SHIFT+< & SHIFT+> can be used to scroll the Kagi chart when it is wider than the terminal (usually only an issue on 5Y timeframes).

Let me know what you think!

alfredo-catalano commented 3 years ago

Very nice, thank you!

miraclx commented 3 years ago

Nice work! Although, shift+left & shift+right is captured by Konsole and Yakuake for switching tabs. Do you think we could perhaps use \< (less than) and > (greater than)?

Or we could ship with these defaults, and people like me with conflicting keybindings could use #65 when it's ready.

EDIT: this works just fine for me in Alacritty.

miraclx commented 3 years ago

I also love that scrolling functionality, it might be very relevant in https://github.com/tarkah/tickrs/discussions/80#discussion-3229364:

Another special feature might be selecting custom time frames, so a ctrl+mouse drag would select a custom range on the x-axis within the current time frame and zoom into that as a custom time frame.

That can be further extended to support L-R scrolling when zoomed in to custom timeframes.


EDIT: just realized you (@tarkah) already made that comment in https://github.com/tarkah/tickrs/discussions/80#discussioncomment-402571

"Zoom" would be a cool phase 2 of this. I have scrolling reliably implemented now (from the Kagi chart) and I think thatd the hard part of doing a zoom.

tarkah commented 3 years ago

Hmm.. that sucks its captured by such a common console. Maybe [ & ] would be sane defaults, or < & > like you said? Or I could have it capture both since I still like shift + arrows.

Edit: Yeah, I think I will go with < & > on top of SHIFT + <- / ->

miraclx commented 3 years ago

That sounds reasonable, tbh I actually like shift+<-/-> too. I'll check if I can free up those bindings to actually be able to use them with tickrs.

Again, love it! and thanks @alfredo-catalano for the chart suggestion.

EDIT: I was able to disable those bindings in both terminals, so it works fine now.

miraclx commented 3 years ago

I guess, (for now), redundancy in the default keybindings can be okay but maybe after #65 (customizable keybindings), we can drop the \<+> and leave it up to the users to configure that if they have a conflict.

tarkah commented 3 years ago

Agreed!

So for the customization piece of Kagi chart with specifying reversals.... I was thinking of either adding a block, similar to the toggle block, with a header that said like 'e' to edit and the fields to edit inside it. OR having 'e' edit in the toggle window and have it open a pane, similar to options pane, that has the fields you can edit. Then that can be expanded on way easier in the future for additional configuration options since its a big pane

miraclx commented 3 years ago

OR having 'e' edit in the toggle window and have it open a pane, similar to options pane, that has the fields you can edit. Then that can be expanded on way easier in the future for additional configuration options since its a big pane

I, personally think this one is better because as you pointed out, it's easier for additional configuration in the future and I think, it keeps the UI less cluttered so it makes for good UX.

tarkah commented 3 years ago

Awesome! I'll start working on that once I can peel my eyes away from this craziness...

image

miraclx commented 3 years ago

Damn!

Anyways, once you're settled, you might wanna look into;

tarkah commented 3 years ago
  • adding kagi into the config: maybe the candle entry can become chart | chart_type with the options line | candle | kagi

  • and also the CLI flag: --candle can become --chart <line|kagi|candle>

Great call!

miraclx commented 3 years ago

Also, I noticed Volumes 'v' gets disabled when kagi is switched to but doesn't get re-enabled (if previously active) when you switch out of it.

tarkah commented 3 years ago

Also, I noticed Volumes 'v' gets disabled when kagi is switched to but doesn't get re-enabled (if previously active) when you switch out of it.

Nice catch, I just fixed that.

tarkah commented 3 years ago

--chart-type / -c CLI option and chart_type: _ config option have been added

tarkah commented 3 years ago

FYI, I am probably migrating to this pattern match for handling keys. There are instances where modifiers show up and I don't care about them (capitalized letters), and with this, I can match on _ for the modifier and only have to specify the KeyCode match once.

    match (key_event.code, key_event.modifiers) {
        (KeyCode::Char('c'), KeyModifiers::CONTROL) => {
            cleanup_terminal();
            std::process::exit(0);
        }
        (_, _) => {}
    }

Edit: A current example is '?'... some terminals have it come across as just KeyCode('?') and no modifiers, others KeyCode('?') and Modifiers::SHIFT. You can see how I have it defined twice currently. This should let me do this just once:

        (KeyCode::Char('?'), _) => {
            app.previous_mode = app.mode;
            app.mode = app::Mode::Help;
            let _ = request_redraw.try_send(());
        }
miraclx commented 3 years ago

Absolutely! I'm all for that! I wanted to point it out with #51 and #71 because I noticed a lot of code duplication for generally useful bindings but ... didn't.

We could extend this to catching the bindings on multiple panes even, to further reduce the duplication.

With this worked out, I can open another PR to scratch an itch I noted down earlier (ability to access useful toggles while in the Options pane)

tarkah commented 3 years ago

Yeah, there are definitely some "global" keybinds that we can match on before dispatching to the handle_events of specific modes. The exception is the AddStock mode since it's text entry based. We can't really have '?' or 'q' as globals there, but we can handle this mode prior to globals prior to all other modes.

And definitely feel free to scratch that itch. The contributions are welcomed and appreciated!

tarkah commented 3 years ago

This is taking forever to implement... but pretty much there (formatting is NOT final)! Wait until the end of the gif and you'll see that an error message comes up when submitting invalid values

2C9cZUUDZW

tarkah commented 3 years ago

Ok, I think I have the config screen in a good state. @miraclx What do you think?

hiTDKN8Tor

Last thing I need to do is add ability to add these to the config file per symbol (hashmap), and source the form from those values if supplied (this will be very easy to do)

EDIT: I guess I need to add toggle between using close prices vs high / low too. Currently it's just using close prices.

alfredo-catalano commented 3 years ago

Looks great! Yes, kagi based on high/low.

alfredo-catalano commented 3 years ago

Cory, nice work...

tarkah commented 3 years ago

@alfredo-catalano thanks!

@alfredo-catalano & @miraclx I'm finished! I've added config persistence for these options, as well as the high / low price type.

I've checked the 5Y Kagi against stockcharts.com for both close and high / low and it appears to be spot on. Let me know what you think!

Here is how you can setup the config....

# Ticker options for Kagi charts
#
# A map of each ticker with reversal and/or price fields (both optional). If no
# entry is defined for a symbol, a default of 'close' price and 1% for 1D and 4%
# for non-1D timeframes is used. This can be updated in the GUI by pressing 'e'
#
# reversal.type can be 'amount' or 'pct'
# price can be 'close' or 'high_low'
#
#kagi_options:
#  SPY:
#    reversal:
#      type: amount
#      value: 5.00
#    price: close
#  AMD:
#    price: high_low
#  TSLA:
#    reversal:
#      type: pct
#      value: 0.08
alfredo-catalano commented 3 years ago

Can the configuration handle a different reversal amount for each timeframe? For example, specify a much smaller percentage amount just for the 1D and 1W.

tarkah commented 3 years ago

Can the configuration handle a different reversal amount for each timeframe? For example, specify a much smaller percentage amount just for the 1D and 1W.

I could add that, but it's definitely a trade off since you'd have to define ALL time frames individually in the config each time. Can you try just popping open the config pane with 'e' and making a quick change when you change time frames?

Let me think if there'd be an elegant way to do it at the time_frame level in the config.

alfredo-catalano commented 3 years ago

I was thinking it would be cool to see each timeframe stacked the way you show different tickers in the summary window. Offer an option to see the timeframes stacked in Summary window. That would be ideal.

tarkah commented 3 years ago

I was thinking it would be cool to see each timeframe stacked the way you show different tickers in the summary window. Offer an option to see the timeframes stacked in Summary window. That would be ideal.

Interesting idea! Can you post a new discussion topic around that?

tarkah commented 3 years ago

@alfredo-catalano I'm going to spoil you, I've got reversal type implemented so it can be defined per time frame OR ALL time frames:

#kagi_options:
#  SPY:
#    reversal:
#      type: amount
#      value: 5.00
#    price: close
#  AMD:
#    price: high_low
#  TSLA:
#    reversal:
#      type: pct
#      value: 0.08
#  NVDA:
#    reversal:
#      1D:
#        type: pct
#        value: 0.02
#      5Y:
#        type: pct
#        value: 0.10

Now when using the Edit 'e' pane, it'll persist those change for JUST the selected time_frame

alfredo-catalano commented 3 years ago

I know talent when I see it...

tarkah commented 3 years ago

@miraclx I backmerged your keybind changes to this branch. Can you please help double check I didn't mess anything up?

The new keybinds for the ConfigureChart is in the same precedence area as DisplayOptions. I just had to add a check against 'c' since we don't want to toggle off the chart type while configuring it.

tarkah commented 3 years ago

@miraclx I think we have this pretty well tested now. Anything else or am I good to merge and release?

miraclx commented 3 years ago

Oh, I'm pretty good on this. +1 from here. Nice work man!

alfredo-catalano commented 3 years ago

Awesome work guys!