Use better names for json editor types

[gnuion]

Hello, I am currently learning ratatui and I find the tutorials very helpful. However, I would change some of the method names, for example, in the json_editor example, I'd rather choose set_focus instead of toggle_editing. And also use Focus instead of CurerntlyEditing.:

-  pub fn toggle_editing(&mut self) {
+  pub fn set_focus(&mut self) {
-      if let Some(edit_mode) = &self.currently_editing {
+      if let Some(edit_mode) = &self.focus {
            match edit_mode {
-             CurrentlyEditing::Key => {
+             Focus::Key => {
// and so on...

[gnuion]

Here is my example:

pub fn toggle_focus(&mut self) {
    let focus = match &self.focus {
        Some(focus) => match focus {
            Focus::Key => Focus::Value,
            Focus::Value => Focus::Key,
        },
        None => Focus::Key,
    };

    self.focus = Some(focus);
}

[joshka]

That seems reasonable. If you want to go more idiomatic rust though, it would be good to make wrong states not able to even be available - e.g. how about making focus a tuple field on the Editing enum variant. I think It might also make sense to rename CurrentScreen to EditMode. So you might end up with something like:

pub fn toggle_focus(&mut self) {
    self.edit_mode = match self.edit_mode {
        EditMode::NotEditing => EditMode::Editing(Focus::Key),
        EditMode::Editing(Focus::Key) => EditMode::Editing(Focus::Value),
        EditMode::Editing(Focus::Value) => EditMode::Editing(Focus::Key),
        EditMode::Exiting => EditMode::Exiting,
    };
}

If you want to make this even more robust, I'd suggest replacing the code in the loop that updates app fields with method calls. E.g. the following:

KeyCode::Enter => {
    if let Some(editing) = &app.currently_editing {
        match editing {
            CurrentlyEditing::Key => {
                app.currently_editing =
                    Some(CurrentlyEditing::Value);
            }
            CurrentlyEditing::Value => {
                app.save_key_value();
                app.current_screen =
                    CurrentScreen::Main;
            }
        }
    }
}

Might become:

KeyCode::Enter => app.submit(),

Would you be interested in making a PR to update the code and doc?

Pinging @Nickiel12 (the original author of this for their thoughts)

[Nickiel12]

I'm fine with these changes, as some of it (especially the "current screen") is just left over from some old bad naming habits. The reason for originally naming it CurrentlyEditing was to emphasize that this isn't some magic "make this enum and it will magically make you edit these fields" but is rather an internal state indicator that the programmer is responsible for keeping track of. However, Focus is much more concise, and once it is first brought up, I believe this point should be apparent to people actually paying attention to the tutorial while they are programming it. Whatever the outcome, this has my ok to do.

[joshka]

Cool - having a bit of a play with some refactoring it gets down to:

fn handle_key(key: KeyEvent, app: &mut App) {
    if key.kind == event::KeyEventKind::Release {
        return;
    }
    match app.mode {
        Mode::NotEditing => match key.code {
            KeyCode::Char('e') => app.start_editing(),
            KeyCode::Char('q') => app.quit(),
            _ => {}
        },
        Mode::Editing(_) => match key.code {
            KeyCode::Char(value) => app.push(value),
            KeyCode::Backspace => app.backspace(),
            KeyCode::Esc => app.stop_editing(),
            KeyCode::Tab => app.toggle_focus(),
            KeyCode::Enter => app.submit(),
            _ => {}
        },
        Mode::Quitting => match key.code {
            KeyCode::Char('y') => app.exit(ExitMode::Print),
            KeyCode::Char('n') => app.exit(ExitMode::NoPrint),
            _ => {}
        },
        _ => {}
    }
}

[gnuion]

Cool, this is interesting, as the edit mode now requires a focus. So you won't need to keep track of the focus directly, you just save it as the context of the mode.
I've chosen a more UI-oriented naming regarding the mode. Instead, I named it View and then the different views as enum variants. Main could be named List instead, as it lists all the entries.

I could make a pull request and suggest my style, but there is no right or wrong. I just found toggle editing the first time I saw it a bit confusing, as it indicated to me as entering and exiting edit mode. But following the code it becomes very clear what it is meant.

pub enum View {
    Main,
    Edit,
    Exit,
}

[joshka]

View definitely sounds better. Here's what I get to with my refactor (anchor comments and imports removed for brevity).

• Added color_eyre, which avoids having to worry about capturing the app result.
• Moved all the code that mutates app into app
• Moved the code that controls what to do on exit into app

Would need to rewrite a bunch of the docs to use this though.

// main.rs
fn main() -> color_eyre::Result<()> {
    init_error_handling()?;
    let mut terminal = init_terminal()?;
    let mut app = App::new();
    run(&mut terminal, &mut app)?;
    restore_terminal()?;
    app.print_json()?;
    Ok(())
}

/// Setup error handling with color_eyre.
///
/// Ensures that if a panic or error occurs, the terminal is restored to its original state prior
/// to showing the messages.
fn init_error_handling() -> color_eyre::Result<()> {
    let (panic_hook, error_hook) = HookBuilder::default().into_hooks();
    let panic_hook = panic_hook.into_panic_hook();
    panic::set_hook(Box::new(move |info| {
        restore_terminal().unwrap();
        panic_hook(info);
    }));
    let error_hook = error_hook.into_eyre_hook();
    color_eyre::eyre::set_hook(Box::new(move |error| {
        restore_terminal().unwrap();
        error_hook(error)
    }))?;
    Ok(())
}

/// Creates a terminal and enables raw mode.
///
/// Uses stderr as the output as the app will print the json to stdout when it exits.
fn init_terminal() -> io::Result<Terminal<impl Backend>> {
    enable_raw_mode()?;
    stderr().execute(EnterAlternateScreen)?;
    let backend = CrosstermBackend::new(stderr());
    let terminal = Terminal::new(backend)?;
    Ok(terminal)
}

fn restore_terminal() -> io::Result<()> {
    disable_raw_mode()?;
    stderr().execute(LeaveAlternateScreen)?;
    Ok(())
}

fn run(terminal: &mut Terminal<impl Backend>, app: &mut App) -> io::Result<()> {
    while app.is_running() {
        terminal.draw(|f| ui(f, app))?;
        if let Event::Key(key) = event::read()? {
            handle_key(key, app)
        }
    }
    Ok(())
}

fn handle_key(key: KeyEvent, app: &mut App) {
    if key.kind == KeyEventKind::Release {
        return;
    }
    match app.view {
        View::Main => match key.code {
            KeyCode::Char('e') => app.edit(),
            KeyCode::Char('q') => app.quit(),
            _ => {}
        },
        View::Edit(_) => match key.code {
            KeyCode::Char(value) => app.push(value),
            KeyCode::Backspace => app.backspace(),
            KeyCode::Esc => app.cancel_edit(),
            KeyCode::Tab => app.toggle_focus(),
            KeyCode::Enter => app.submit(),
            _ => {}
        },
        View::Quit => match key.code {
            KeyCode::Char('y') => app.exit(ExitAction::Print),
            KeyCode::Char('n') => app.exit(ExitAction::NoPrint),
            _ => {}
        },
        _ => {}
    }
}

// app.rs
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum View {
    Main,
    Edit(Focus),
    Quit,
    Exit(ExitAction),
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ExitAction {
    Print,
    NoPrint,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Focus {
    Key,
    Value,
}

pub struct App {
    pub key_input: String, // the currently being edited json key.
    pub value_input: String, // the currently being edited json value.
    pub pairs: HashMap<String, String>, // The representation of our key and value pairs with serde Serialize support
    pub view: View, // the current screen the user is looking at, and will later determine what is rendered.
}

impl App {
    pub fn new() -> App {
        App {
            key_input: String::new(),
            value_input: String::new(),
            pairs: HashMap::new(),
            view: View::Main,
        }
    }

    pub fn save_key_value(&mut self) {
        self.pairs
            .insert(self.key_input.clone(), self.value_input.clone());

        self.key_input = String::new();
        self.value_input = String::new();
    }

    pub fn toggle_focus(&mut self) {
        self.view = match self.view {
            View::Main => View::Edit(Focus::Key),
            View::Edit(Focus::Key) => View::Edit(Focus::Value),
            View::Edit(Focus::Value) => View::Main,
            other => other,
        };
    }

    pub fn print_json(&self) -> Result<()> {
        if self.view != View::Exit(ExitAction::Print) {
            return Ok(());
        }
        let output = serde_json::to_string(&self.pairs)?;
        println!("{}", output);
        Ok(())
    }
    // ANCHOR_END: print_json

    pub fn edit(&mut self) {
        self.view = View::Edit(Focus::Key);
    }

    pub fn cancel_edit(&mut self) {
        self.view = View::Main;
    }

    pub fn quit(&mut self) {
        self.view = View::Quit;
    }

    pub fn submit(&mut self) {
        match self.view {
            View::Edit(Focus::Key) => self.view = View::Edit(Focus::Value),
            View::Edit(Focus::Value) => {
                self.save_key_value();
                self.view = View::Main;
            }
            _ => {}
        }
    }

    pub fn backspace(&mut self) {
        match self.view {
            View::Edit(Focus::Key) => self.key_input.pop(),
            View::Edit(Focus::Value) => self.value_input.pop(),
            _ => None,
        };
    }

    pub fn push(&mut self, c: char) {
        match self.view {
            View::Edit(Focus::Key) => self.key_input.push(c),
            View::Edit(Focus::Value) => self.value_input.push(c),
            _ => {}
        };
    }

    pub fn is_running(&self) -> bool {
        match self.view {
            View::Exit(_) => false,
            _ => true,
        }
    }

    pub fn exit(&mut self, action: ExitAction) {
        self.view = View::Exit(action);
    }
}

[gnuion]

Awesome! Now another thing that is confusing to me is the name tick_rate. Usually, something with higher rate happens more often. But for the tick_rate the opposite is true. It is more like a duration instead of a rate. Because the longer the tick duration, the less often the tick occurs, therefore it is the inverse of the rate.

[gnuion]

So, looking at the ratatui-async-template, they use a float for the number of ticks in a second. And then for calculating the delay between the ticks, they use the following formula:

let tick_rate = 4.0;
...
let tick_delay = std::time::Duration::from_secs_f64(1.0 / self.tick_rate);

[joshka]

I agree, but I go a little further on this. My perspective is that tick is really not useful for the demo apps, and so it's difficult to put in without an obvious reason. It's only useful if there's actual work to do that is not the main part of the loop (like handling animations / spinners / or polling resources on a fixed schedule). Otherwise the app doesn't really need a timeout at all.

• https://discord.com/channels/1070692720437383208/1179680217120383007

[joshka]

As an aside, As a new user I'd encourage you to have a read of the rewritten counter apps in #209 and let me know what you think of the changes - as these are likely to flow through to the entire set of tutorials.

[gnuion]

Thank you joshka. Okay, I will orient my feedback towards the rewrite. Right off the bat, the dark theme colors are easier on the eye and I find the increased contrast easier to read.

[joshka]

Right off the bat, the dark theme colors are easier on the eye and I find the increased contrast easier to read.

The brown? That's actually the old theme, which I changed recently on the main site. Can you screencap the differences (on #209)