Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: inputs command to display a palette menu with all user defined inputs #665

Merged
merged 5 commits into from
Oct 6, 2023

Conversation

tomasklaen
Copy link
Owner

image

@tomasklaen
Copy link
Owner Author

is '%s doesn\'t exist' the best message here? Maybe it should be %s file is missing, or only %s is missing so it's more versatile and can be used in other places in future potentially.

@christoph-heinrich
Copy link
Contributor

%s not found is also an option.

@christoph-heinrich
Copy link
Contributor

That could probably be done with a single line:match() instead of two and then checking if the key starts with a # and if the comment is an empty string, but that's not performance critical so whatever.

Is the menu item parsing code still exactly the same as before?

@tomasklaen
Copy link
Owner Author

tomasklaen commented Oct 3, 2023

That could probably be done with a single line:match() instead of two and then checking if the key starts with a # and if the comment is an empty string, but that's not performance critical so whatever.

I've tried, but after running into issues (like the original won't match lines that don't end with a comment), I've realized that the resulting pattern and subsequent handling of its outputs would be too hacky, lost patience, and just split it into a separate match so it's simple and clear what's going on in each. It's not like there's a risk of this becoming a performance issue.

Is the menu item parsing code still exactly the same as before?

Yes.

%s not found is also an option.

Yeah I guess this is the most versatile one.

@christoph-heinrich
Copy link
Contributor

I've tried, but after running into issues (like the original won't match lines that don't end with a comment), I've realized that the resulting pattern and subsequent handling of its outputs would be too hacky, lost patience, and just split it into a separate match so it's simple and clear what's going on in each.

Yeah I already suspected that's what happened, parsing can be surprisingly tricky at times.

@christoph-heinrich
Copy link
Contributor

christoph-heinrich commented Oct 3, 2023

I've now tried it and it already works well, but by parsing input.conf we miss out on keybindings from other scripts.
A lot of scripts have a hard coded keybinding at the bottom of the script and those won't be listed with this approach.
The same goes for the builtin keybindings.

Alternatively we could do something like stats.lua.


Another thing not directly related to this is that very long titles can end up cutting off even short hints because of the way the space gets divided.
That is from a fullscreen window and even though the title takes up almost the entire screen width it still cuts off the hint.
image
We should cap that somehow, but a stupid 10%-90% width cap isn't good enough, because if one is very short the other one should be able to take up the space up to that point.

And dividing it up linearly is also not really optimal, shorter strings should have get a higher weight per length then longer ones. Some sort of sigmoid, but mirrored around the 45 degree axis and going from 0 to 1 instead of infinity in both directions. Something with arcsin maybe? I'll make a PR when I come up with something useful.

@tomasklaen
Copy link
Owner Author

I had no idea input-bindings was a thing 😐. That makes way more sense than parsing a conf file. I'll change it.

@christoph-heinrich
Copy link
Contributor

christoph-heinrich commented Oct 4, 2023

The code looks and works fine, but sorting the items in some way would be nice. Currently there is no order at all.
At first I though about sorting based on they key(s), but stats.lua groups them nicely.
I haven't looked at how it's doing that, but it's pretty neat.

@tomasklaen
Copy link
Owner Author

Yes, the order from input-bindings is pretty much random. Don't want to do much more here, as this is primarily supposed to be searchable, but sorting at least by title puts items that are somewhat related next to each other.

@tomasklaen tomasklaen merged commit 473278c into main Oct 6, 2023
@tomasklaen tomasklaen deleted the inputs_menu branch October 9, 2023 12:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants