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

Bugs in v0.9.0-pre #234

Closed
Builditluc opened this issue Nov 3, 2024 · 21 comments
Closed

Bugs in v0.9.0-pre #234

Builditluc opened this issue Nov 3, 2024 · 21 comments
Assignees
Labels
type: bug This fixes a bug. Increment the minor version
Milestone

Comments

@Builditluc
Copy link
Owner

This issues tracks the found bugs in the 0.9.0-pre development version

@Builditluc Builditluc added the type: bug This fixes a bug. Increment the minor version label Nov 3, 2024
@Builditluc Builditluc added this to the Version 0.9.0 milestone Nov 3, 2024
@Builditluc Builditluc self-assigned this Nov 3, 2024
@0323pin
Copy link
Contributor

0323pin commented Nov 3, 2024

I've missed the new issue and kept writing on the other one. Sorry.

It was simple to follow and create a theme.

# theme.toml

bg                 = "reset"
fg                 = "reset"
title              = "white"
search_title_fg    = "red"
selected_bg        = "darkgray"
selected_fg        = "reset"
inactive_fg        = "green"
highlight_fg       = "white"
scrollbar_track_fg = "black"
scrollbar_thumb_fg = "yellow"
border_fg          = "white"
border_bg          = "reset"
border_type        = "Rounded"

Only minor changes from defaults.

But, the config file seems to be harder to find. Right now only:

# config.toml

api.language      = "en"
logging.log_level = "Error"

Would you happen to have a more comprehensive one?
TIA

@Builditluc
Copy link
Owner Author

Right now, configuration (apart from theming) is not implemented, meaning the options from the dev documentation (not regarding theming) don't exist.

I'm still unsure on how to structure the configuration (previously, we were limited on how to structure the config, but now we have complete freedom on how to lay out / name things).

The old configuration layout mostly made sense (keybindings, logging, toc) but the api settings and features especially I find a bit confusing (e.g. why would you disable the toc with features.toc = false and not with settings.toc.enabled = false where to other configuration options for the toc lay?)

@0323pin
Copy link
Contributor

0323pin commented Nov 3, 2024

Right now, configuration (apart from theming) is not implemented ... now we have complete freedom on how to lay out / name things

This is perfectly fine. I don't think I would like to change everything like, mess with layout.
log_level and keybindings, that's what I had in mind.

Everything is working smooth and I haven't come across any additional bugs apart from the ones you quickly fixed :)
Working great!

@0323pin
Copy link
Contributor

0323pin commented Nov 3, 2024

you can change the logging level by setting the WIKI_TUI_LOG env variable to whatever level you want

I can use this, no problem.

So, perhaps the config file should just allow to override default bindings.

@Builditluc
Copy link
Owner Author

Builditluc commented Nov 3, 2024

I don't think I would like to change everything like, mess with layout.

I mean we can add some basic configuration options like disabling borders. But if someone really wants to complete reorder the layout, they can create an issue themselves

@0323pin
Copy link
Contributor

0323pin commented Nov 3, 2024

So I'm reading src/logging.rs and can only see two options for the level 'WARN' and 'DEBUG'

If, I'm defining this as an environment variable, I need the correct naming (case sensitive).

I use elvish as my default shell, so what are the options here?

set-env WIKI_TUI_LOG "???"

Is it WARN or, DEBUG?

@Builditluc
Copy link
Owner Author

Builditluc commented Nov 3, 2024

So I did some digging to find the correct values. Basically we use the EnvFilter to retrieve the log level from the env variable. The Filter itself is very powerful (allowing use to theoretically use directives to set the log level for differnet modules/packages/etc), but we only use it to parse the env. From the documentation, it accepts any level the default LevelFilter from tracing, and when you look at its FromStr implementation, those should be the values accepted in the env variable

@0323pin
Copy link
Contributor

0323pin commented Nov 3, 2024

Thanks! I'll set this on my rc.elv ... all capitals :)

@Builditluc
Copy link
Owner Author

Builditluc commented Nov 3, 2024

I've now finished documenting the unreleased changes and put them in the changelog. Also moved the changelog to docs/docs/changelog/index.md (where the documentation site resides) and created a symbolic link to it.

EDIT: You can view the changelog now on the site https://wiki-tui.net/dev/changelog/

@0323pin
Copy link
Contributor

0323pin commented Nov 3, 2024

Implemented the latest changes to the status bar. Great stuff!

2024-11-03-165038_1366x768_scrot

@Builditluc Builditluc pinned this issue Nov 3, 2024
@Builditluc
Copy link
Owner Author

A little annoying bug to note: Sometimes the app won't "let" you scroll down (keeps jumping back up when scrolling), but when you go down completely or jump to a header, scrolling works just fine again (even at the previous position). If I'm able to reliably reproduce the issue, I can work on a fix

@0323pin
Copy link
Contributor

0323pin commented Nov 4, 2024

Haven't hit that, yet but, it does sound annoying and most probably not straightforward to find what's causing it.

@0323pin
Copy link
Contributor

0323pin commented Nov 7, 2024

@Builditluc could I ask for some (to me very useful) things?

Could you please include MSRV by declaring the required rust version in the main Cargo.toml file?

Also, you might want to bump edition to 2021. Please don't bump it to 2024, yet. This would break builds on NetBSD for now.

TIA

@Builditluc
Copy link
Owner Author

Builditluc commented Nov 8, 2024

Also, you might want to bump edition to 2021

What are we gaining from updating the version to 2021 from 2018? I would just have kept it until it no longer works or we need features from future editions.

Could you please include MSRV by declaring the required rust version in the main Cargo.toml file?

Yeah sure, currently we have that defined in rust-toolchain but I don't see any point in not including it in the mainfest. I've gone ahead and determined the msrv using cargo-msrv, which is 1.76.0.

I'll create the necessary commits for adding the msrv to the manifest (6574f7e)

@0323pin
Copy link
Contributor

0323pin commented Nov 8, 2024

Thank you!

@Builditluc
Copy link
Owner Author

Small little issue: You cannot scroll horizontally in the toc and when a section is too long, it just gets cut off (no ... is added).

Will fix that too

@Builditluc
Copy link
Owner Author

There is some weird behaviour going on with the searchbar when having a page in zenmode (if you go back to the search while still in zen-mode using s, the searchbar is still hidden)

@0323pin
Copy link
Contributor

0323pin commented Dec 3, 2024

I would never find this since I have the search bar on, even on zen mode

# config.toml

page.toc.width_percentage = 25
page.zen_mode.include     = "SEARCH_BAR | SCROLLBAR | STATUS_BAR"
page.padding              = 2

[bindings.global]
scroll_down               = "down"
scroll_up                 = "up"

@Builditluc
Copy link
Owner Author

Builditluc commented Dec 4, 2024

I think I found the issue. Looking at the backtrace when the issue occurs, there looks to be some sort of recursion going on that in some cases causes weird behavior with scrolling.
image
EDIT: You can see that scroll_to_y is called with the correct y value and it then calls itself with y=0 which causes a jump to the top

@0323pin
Copy link
Contributor

0323pin commented Dec 4, 2024

Awesome 🚀

I'll rebuild from head in a few minutes.

@Builditluc
Copy link
Owner Author

Closing this as v0.9.0 has been released

@github-project-automation github-project-automation bot moved this from Todo to Done in wiki-tui: Roadmap Dec 4, 2024
@Builditluc Builditluc unpinned this issue Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug This fixes a bug. Increment the minor version
Projects
Status: Done
Development

No branches or pull requests

2 participants