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

Refactor UI components to be more modular #2

Closed
Samyak2 opened this issue Feb 27, 2022 · 1 comment · Fixed by #4
Closed

Refactor UI components to be more modular #2

Samyak2 opened this issue Feb 27, 2022 · 1 comment · Fixed by #4
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Samyak2
Copy link
Owner

Samyak2 commented Feb 27, 2022

What?

Currently, the terminal UI in toipe is rendered by directly writing to stdout with cursor positioning and colors provided by termion.

Example:

toipe/src/lib.rs

Lines 263 to 271 in fae3005

write!(
self.stdout,
"{}{}{}{}{}",
cursor::Goto(sizex / 2, sizey / 2 - 2),
cursor::Left(line.len() as u16 / 2),
color::Fg(color::Blue),
line,
color::Fg(color::Reset),
)?;

This code looks a bit ugly and we need to manually position items by changing the arguments given to cursor::Goto. For example, sizey / 2 - 2 means two lines above the middle line. If I want to add another line to that list and move all of them down, I need to change this part of every line.

The cursor::Left(line.len() as u16 / 2) part positions the text in the center - this snippet is repeated for every line. What's even worse is if the text in that line includes special characters such as colors, this will not work because it will consider those hidden characters too! To get around that, I use this hack:

toipe/src/lib.rs

Lines 285 to 299 in fae3005

let line = format!(
"Speed: {}{:.1} wpm{} (words per minute)",
color::Fg(color::Green),
results.wpm(),
color::Fg(color::Reset)
);
// do not consider length of formatting characters
let zerowidths = format!("{}{}", color::Fg(color::Green), color::Fg(color::Reset));
write!(
self.stdout,
"{}{}{}",
cursor::Goto(sizex / 2, sizey / 2),
cursor::Left((line.len() - zerowidths.len()) as u16 / 2),
line,
)?;

As you can see, I need to repeat the formatting characters again in a separate string containing only them and then subtract their length from line.len() like cursor::Left((line.len() - zerowidths.len()) as u16 / 2).

How to fix?

Honestly, I'm not sure. I opened this issue to force myself to document the problem.

The solution will definitely involve abstracting this UI rendering into a different module. Abstracting the cursor::Gotos is simple - take a vec/iter/slice of strings and change the y-position when printing each one such that they are approximately in the middle. cursor::Left(line.len() as u16 / 2) is easy too, as long as the given string contains single-width characters. I'm not sure how zero-width and other characters can be handled here. Perhaps this problem is solved elsewhere?

I'm always open to suggestions!

@Samyak2 Samyak2 added enhancement New feature or request help wanted Extra attention is needed labels Feb 27, 2022
Samyak2 added a commit that referenced this issue Mar 2, 2022
Fixes #2

Added a new struct `Text`:
- abstracts formatting and coloring in such a way that length is
  preserved i.e., zero-width characters are not counted in the length
    - uses a new trait `HasLength` which gives a `length()` method to
      get the actual length
    - `HasLength` is also implemented for a slice of Text
- implemented conversions from String, &str, char

Added a new struct `ToipeTui`:
- stores the stdout terminal ui (from termion)
- moved flush and reset_screen to this
- added methods to display a raw `Text`, a line of text (which can include
  multiple disjoint `Text`), multiple lines (with each line having
  multiple `Text`).
- resetting terminal on dropping is handled by this

Changed the `Toipe` struct to use `ToipeTui` methods when possible:
- displaying words at the beginning uses `display_a_line`
- displaying green/red chars on typing uses `display_raw_text`
- displaying results uses `display_lines` to abstract away the special
  handling of zero-width characters and the aligning of text
    - The text formatting part looks a little worse, but everything
      seems much cleaner!
- added TODOs in places where ToipeTui could be further used instead of
  accessing the stdout directly

also added TODOs for docstrings
@Samyak2
Copy link
Owner Author

Samyak2 commented Mar 3, 2022

This is in-progress in https://github.com/Samyak2/toipe/compare/ui-refactor

The formatting code is a bit cumbersome to use. Maybe a macro similar to format! would make sense here.

@Samyak2 Samyak2 mentioned this issue Mar 4, 2022
@Samyak2 Samyak2 closed this as completed in #4 Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant