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

Tags not showing on last entry. #158

Closed
metalsp0rk opened this issue Apr 4, 2021 · 13 comments
Closed

Tags not showing on last entry. #158

metalsp0rk opened this issue Apr 4, 2021 · 13 comments
Labels
bug Something isn't working good first issue Good for newcomers Stale
Milestone

Comments

@metalsp0rk
Copy link

Describe the bug
Tags are not showing for the last entry in ticker, when scrolling is applicable.

To Reproduce
Steps to reproduce the behavior:

  1. launch with --show-tags
  2. Scroll to the bottom
  3. Observe that the last entry does not show the tags.

Expected behavior
The interface will allow you to scroll down to the very bottom.

Screenshots
image

Environment (please complete the following information):

  • OS: Linux
  • Terminal: st
  • Terminal Version: 0.8.4
  • Font: (Optional) [e.g. Powerline]
  • ticker Version: 3.0.7

Additional context
If you resize the terminal window to increase the vertical size, the tags will show up, but if you scroll up one line, then try to scroll back down, it will not allow you to.

I suspect the key-scroll method has a bounds-check error. I'll dig into it when I have a chance and if I can figure it out, I'll submit a PR. Just thought I'd submit an issue to share my findings.

@metalsp0rk metalsp0rk added the bug Something isn't working label Apr 4, 2021
@sergeevabc
Copy link

@metalsp0rk, you’re lucky, in my case ticker.exe --show-tags displays a black screen with clock ticking, that’s all.

@metalsp0rk
Copy link
Author

oh wow, I'm not sure what's going on there. You're on windows, I assume. I don't have access to a windows system to verify your error, so I'm not sure how much I'll be able to do about this.

However, are you using cmd or powershell to get this result? I'll see if I can get a coworker to confirm this.

@achannarasappa
Copy link
Owner

Thanks for reporting this @sergeevabc ! Looks this bug has re-emerged in a recent release.

Would very much appreciate a PR if you end up having time to take a look but if not I'll take a look at fixing it myself at some point.

@achannarasappa achannarasappa added this to the future-release milestone Apr 5, 2021
@sergeevabc
Copy link

@achannarasappa, my profile says “bugs hunter”, while PR requires developer skills and tools. I’ll wait and verify when it’s ready.

@achannarasappa
Copy link
Owner

Meant to tag the author in this case @metalsp0rk :)

In any case thank you both for your input and details on the issue

@metalsp0rk
Copy link
Author

I haven't yet had time to look into it, if anyone else wants to take lead on this, I'm fine with that. Otherwise, I'll take a look when I get some time.

@achannarasappa achannarasappa added the good first issue Good for newcomers label Apr 15, 2021
@achannarasappa achannarasappa modified the milestones: future-release, v4.2.0 May 31, 2021
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@MarcoLucidi01
Copy link

hello, I'm a big fan of ticker, I use it daily to track my positions. I saw this "good first issue" and decided to help :)


what I found out is basically that when the content doesn't fit on a single page, github.com/charmbracelet/bubbles/viewport doesn't render the last line of content if it doesn't end with a new line \n character (bubbles/viewport is used in ticker's ui.Model).

I was able to reproduce the issue even with bubbletea's pager example that uses bubbles/viewport. here what I did:

$ git clone https://github.com/charmbracelet/bubbletea.git
...
$ cd bubbletea/examples/pager/
$ go build
$ ls -1
artichoke.md    # this file is displayed in the example
main.go
pager*
$ tail -1 artichoke.md | xxd | tail -1
00000030: 2069 6e20 5370 616e 6973 682e 0a          in Spanish..    # ends with new line
$ ./pager
lastline.mp4

everything looks fine, now I remove last new line \n character from artichoke.md:

$ truncate -s -1 artichoke.md
$ tail -1 artichoke.md | xxd | tail -1
00000030: 2069 6e20 5370 616e 6973 682e             in Spanish.
$ ./pager
nolastline.mp4

and the last line

_Alcachofa_, if you were wondering, is artichoke in Spanish.

is not displayed anymore. the same thing is happening in ticker where the last line is the "tags line". as you can imagine, the issue is not directly related to --show-tags flag: it can happen with any "last line" not ending with a new line \n.

a quick-and-dirty fix for ticker would be to add a terminating new line \n to watchlist.View() (viewport's content), e.g.:

$ git diff
diff --git a/internal/ui/component/watchlist/watchlist.go b/internal/ui/component/watchlist/watchlist.go
index 00bdebe..65d56da 100644
--- a/internal/ui/component/watchlist/watchlist.go
+++ b/internal/ui/component/watchlist/watchlist.go
@@ -105,7 +105,7 @@ func (m Model) View() string {

        }

-       return grid.Render(grid.Grid{Rows: rows, GutterHorizontal: widthGutter})
+       return grid.Render(grid.Grid{Rows: rows, GutterHorizontal: widthGutter}) + "\n"
 }

 func getCellWidths(assets []c.Asset) cellWidthsContainer {

this works, but it's ugly. I'm pretty positive that the bug is in bubbles/viewport rather than in ticker. I think that not rendering the last line of content when it doesn't end with a new line \n is not expected behavior.

I tried to dig into bubbles/viewport/viewport.go, there are lot of places where they do len(m.lines)-1 i.e. subtract 1 from total number of lines. I didn't quite understand why it's necessary, but I suspect it's related to the bug.

anyway, I was thinking about opening an issue in bubbles/viewport reporting all this, what do you guys think?

@achannarasappa
Copy link
Owner

Thanks for doing that analysis and finding the likely source of the issue @MarcoLucidi01!

It sounds like the ideal place to make the fix would be bubbles/viewport and I'd suggest starting there. I'm eager to get this bug resolved as it's sitting unresolved for a while now and glad you were able to provide a lot more clarity on the problem.

@MarcoLucidi01
Copy link

quick update: the bug has been fixed in bubbles/viewport's master and will be available in the next bubbles release:

charmbracelet/bubbles#74
charmbracelet/bubbles#73 (comment)

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 20, 2021
@github-actions
Copy link

github-actions bot commented Dec 4, 2021

This issue was closed because it has been stalled for 14 days with no activity.

@github-actions github-actions bot closed this as completed Dec 4, 2021
@meowgorithm
Copy link

This fix is now available in Bubbles v0.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers Stale
Projects
None yet
Development

No branches or pull requests

5 participants