/ to redefine glob at runtime

[Davejkane]

Goal

Add an extra handle for the / (because it's used for search in vim) to allow redefining the glob at runtime.

Implementation

This will require the following to be taken into consideration:

• / key to start taking keyboard input as a string.
• enter or return key to indicate end of input.
• update the glob and reproduce the images paths
• determine if the current image path exists in the new list of paths. If it does, update the index to it's new index, otherwise set the index to zero.

As for the text input, we will probably need to use https://rust-sdl2.github.io/rust-sdl2/sdl2/keyboard/struct.TextInputUtil.html.

Although this tutorial is in C, it appears very relevant. https://wiki.libsdl.org/Tutorials/TextInput

This video also contains some caveats about backspace. https://www.youtube.com/watch?v=m2doh3Li65c

[NickHackman]

This seems really cool, I'm game to work on this if no one else is.

Really like the feature list so far, but I think an interesting addition would be to take the vim roots and have, if the provided file is in images jump to that index that would be convenient in addition to the other features you've outlined.

[gurgalex]

I don't see that anybody else is assigned to the issue.
@NickHackman It's yours then.

[gurgalex]

@NickHackman
Hmm, A search option? (with possible multiple matches).

#48
Need for Having different actions based on the state of the app or part of text currently being entered.
An example, cancelling a search with Esc should not quit the app.

[NickHackman]

@gurgalex Easy solution would be just to use relative paths to prevent multiple matches, but man it would be way cooler if we used 'n', or 'N' (backwards) like in vim to move between the multiple matches.

[gurgalex]

Having both would be nice. redefine glob and searching for matches (without redefining global glob)

I really did want a config file or something to change default keybindings for this case (user prefs).

Hmm, can yo make an issue detailing the proposed feature (search) and a good keyboard default for it. Unsure if (Shift + /) (?) could be a good one.

[Davejkane]

I'll write up a spec for the issue you describe. As for keybindings, perhaps slash for what you describe would be best. Then this feature could use s for search?

[gurgalex]

I think this is what we're both thinking.

issue I described: configurable keyboard shortcuts via config

Glob redefine: /

search for image in current glob: s

[NickHackman]

@Davejkane After spending a good amount of time learning sdl2 more closely, I hit a roadblock with implementing and the potential solution I have, I think, should need approval. So in order to properly get the user input I need to be able to drive the event loop as seen here more than one event loop can't exist and would probably be catastrophic if it could.

Note: when I say drive I actually mean own/ownership drive just sounds more fun.

As of right now I'm just following suit of Action inside of run I'd have to drive the event loop, but obviously can't due to it already being mutably borrowed. Unless I'm missing something here if so I'm sorry for the trouble.

Potential solution: go towards a modal flow of command

Changes:

• Current run -> run_normal_mode
• Create enum and have program cache its current mode

pub enum Mode {
    Normal, // [default] (current run)
    Command, 
    // additional modes would be an easy addition
    Exit,
}

In the current situation Exit would only be called from normal. This would allow the ownership of event loops to be changed. Allowing user input to be gathered for #40, #53, maybe even an add image/glob feature as well (future) and a further progression towards a Vim ideal.

• Change the run function to be a loop that just calls run_normal_mode or run_command_mode based on self.Mode and exiting when provided Mode::Exit, having normal mode dictate the when to switch to command, and just exiting command mode either upon enter, escape, or when the user deletes every character in the command buffer

I think this is a large enough change of how the app functions that I don't feel okay just opening a pull request with this and drop the bomb on you and @gurgalex without expressing my intentions.

Extra Features

• When in command mode remove the image path from infobar, and replace it with the input in command buffer
• Add auto completion for file system paths (this could also be done in the future)

Future Ideas

As mentioned above add would be a good addition in the future, so maybe a change to a more general input like : in vim so the syntax would be

:add glob file glob file

or

:a glob file glob file

Keybinding Modifications to Current Issue

This makes me also think changing the current keybindings for search/change_glob to something that follows the above add would be consistent and easy to use for instance search being

:s file

or

:search file

I could also see using the traditional / that vim uses for search rather than changing the current glob
as of right now what would be a good name to change glob is escaping me, but tell me what you think. 😃

[gurgalex]

@NickHackman
We had the same idea roughly!

pub struct Action {
    //~snip~//
    /// Put number on Screen for Numeric operations
    NumericOp(u32),
    // ~snip~//
}
/// The state of the Input processor. Determines
/// Different actions are taken on the same key depending on the StateEntry
pub enum StateEntry {
    /// Process input normally
    Normal,
    /// Process when expecting numeric commands
    Numeric,
}

pub fn event_action(state: &mut State, event: &Event) -> Action {
    //~snip~//
    match event {
    //~snip~//
        Event::KeyDown {
            keycode: Some(k),
            keymod: m,
            ..
        } => match (k, m) {
            (_, &Mod::NOMOD) => match k {
                // NOMOD are numbers on numpad
                Num1 | Num2 | Num3 | Num4 | Num5 | Num6 | Num7 | Num8 | Num9 | Num0 | Kp0 | Kp1
                | Kp2 | Kp3 | Kp4 | Kp5 | Kp6 | Kp7 | Kp8 | Kp9 => {state.input_state = StateEntry::Numeric; process_numeric(state, event)},
    //~snip~//
}

pub struct State {
    /// render_infobar determines whether or not the info bar should be rendered.
    pub render_infobar: bool,
    /// render_help determines whether or not the help info should be rendered.
    pub render_help: bool,
    /// Should the image shown be shown in actual pixel dimensions
    pub actual_size: bool,
    /// Tracks fullscreen state of app.
    pub fullscreen: bool,
    /// Input Event processing state
    pub input_state: StateEntry,
    /// Unprocessed text
    unprocessed_input: String,

}

[gurgalex]

Maybe a name like NewGlob?

[gurgalex]

Found a great article on state machines for some ideas.
https://hoverbear.org/2016/10/12/rust-state-machine-pattern/

[NickHackman]

Maybe a name like NewGlob?

Yeah that's good abbreviated to ng for short, Once we pick a case, in this case Camel, we should stick with it.

Oh, also on #48 probably a good idea to follow vim and display in the far right of the infobar all keys pressed. Really like how that's coming along 👍

[gurgalex]

Oh, didn't know vim even had that.
All this time I thought it was invisible.

[gurgalex]

Thought NewGlob was for the Action variant.

Looks like lowercase is common for vim commands. Along with abbreviations (and shortened names)
Examples
https://www.fprintf.net/vimCheatSheet.html

:e[dit]
:reg[isters] {arg}

So, ng and newglob perhaps.

[gurgalex]

Mode or InputMode sounds like a good name for tracking which input mode we are interpretting events for.

[NickHackman]

Sweet, newglob/ng it is

I think Mode makes the most sense in this context

[gurgalex]

Only having the top level Mode (first upon program open) capture KeyDown events seems useful. Capturing TextInput events and ignoring most keypress events will be useful it seems for states where we are not in the program's main input handler state. Plus Capital letters are one event. https://love2d.org/forums/viewtopic.php?t=81729

…

On Thu, May 16, 2019, 20:19 Nick Hackman ***@***.***> wrote: Sweet, newglob/ng it is I think Mode makes the most sense in this context — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40?email_source=notifications&email_token=AG5ANRCZW4ETND63Z6PAGOTPVYB2PA5CNFSM4HMKN2GKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVTOWQA#issuecomment-493284160>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG5ANRDOCKMDATBGLZPMIYTPVYB2PANCNFSM4HMKN2GA> .

[NickHackman]

Only having the top level Mode (first upon program open) capture KeyDown events seems useful.

Not sure I understand what you mean, if you're referring to normal (default) mode it for the most part captures only KeyDown events.

Yeah, as of right now I'm only capturing Escape/Return/Enter/Return2, everything else is being captured by TextInput, on second thought it should also be handling resizing where we need to rerender.

[gurgalex]

Mostly a heads up that typing "/" generates a KeyDown, TextInput and a KeyUp event.

…

On Fri, May 17, 2019, 01:03 Nick Hackman ***@***.***> wrote: Only having the top level Mode (first upon program open) capture KeyDown events seems useful. Not sure I understand what you mean, if you're referring to normal (default) mode it for the most part captures only KeyDown events. Yeah, as of right now I'm only capturing Escape/Return/Enter/Return2, everything else is being captured by TextInput, on second thought it should also be handling resizing where we need to rerender. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40?email_source=notifications&email_token=AG5ANREQ4PKZMS76EQS3LVLPVZDCXA5CNFSM4HMKN2GKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVT2JOQ#issuecomment-493331642>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG5ANRGWQWOLOO44FEGURI3PVZDCXANCNFSM4HMKN2GA> .

[Davejkane]

This all seems very cool, and pretty interesting. I have no issue with the way you're solving it.