Support query keyword in activateBookmark.

[mijoharas]

Description

I would like to create a shortcut that lets me browse a specific folder in my bookmarks. To do this I would like to prefill a query into the Vomnibar.activateBookmark command.

What I want is to have:

map wtf Vomnibar.activateBookmarks query=/links

To allow me to browse the links folder of my bookmarks.

It turns out that those options already get parsed into the keyregistry, and the activate command already accepts these options, so the only thing to needed would be to add this extra argument to the content script (similarly to the Vomnibar.activate above) as is done in the PR.

[philc]

This is a cool feature. I've tested your PR and it works.

However, I'm realizing that one can't pass a query with a space in it using the map keyword in one's Vimium config, unless I'm mistakenly missing some syntax that's available. Neither %20, \ , nor + works.

This feature is still useful, because the query /one will prefix match a folder named "one two". But it's not ideal: if a user has a space somewhere in the folder chain of their bookmarks, they can't precisely query for that folder, only the prefix prior to the space.

Before merging this and documenting the feature, I think we'll need a syntax for encoding spaces in options passed to commands with map.

FYI @UncleSnail since you've been working on command syntax recently.

[mijoharas]

Good point, I'd not noticed that.

So, if we wanted to just do a quick substitution of the parsed options we could do the following for %20 going to space.
background_scripts/commands.js:210

      options[parse[0]] = parse.length === 1 ? true : parse[1].replace("%20", " ");

Also, while messing around with that, I noticed that the unit tests are broken. I fixed them in a commit, which I'm gonna push to this branch (might be cleaner to split it off into it's own PR, but I'm not sure how you want to do things for this)

Actually while I"m at it I'm also gonna push a commit and test for changing %20 to space. Feel free to cherry pick this stuff (i.e. fixing the tests) and/or change the approach (we obvs didn't settle on a syntax that was wanted) ofc, just figure I'll throw it up here for comment while it's on my machine/in my head.

[philc]

@mijoharas Ack, sorry about the unit test breakage. That was a subtle breakage (make.js still exits successfully...). I've pushed a fix in bf0bdda.

I'm not sure %20 is a good solution for how to handle spaces; it's just one thing that I tried. If we decode %20 in option values, does that mean we decode every other escape sequence? Supporting some kind of quoting around option values is another solution. I haven't used Vim in years, but I think .vimrc supports triple backslash for escaping spaces, and also surrounding option values with single quotes, for instance.

Let's see if anyone else chimes in with an opinion.

[mijoharas]

yeah, I don't particularly think that %20 is a very nice solution either, (mostly just pointing out where we could do a quick fix if we wanted) nor do I use vim so I don't have much to suggest, so let's wait for someone else to chime in.

[UncleSnail]

Supporting some kind of quoting around option values is another solution. I haven't used Vim in years, but I think .vimrc supports triple backslash for escaping spaces, and also surrounding option values with single quotes, for instance.

Let's see if anyone else chimes in with an opinion.

Thank you for pinging me on this @philc. I was thinking the same as you suggested here. I need to escape spaces in bash nearly every day, and while I occasionally write the bash command (if it's short) like:

command -d option\ with\ spaces

It is usually easier to use:

command -d "option with spaces"

For the average user, I believe this would be the easiest and most understandable solution. If we want an option with spaces, just write it in double quotes:

map asdf Vomnibar.activateBookmarks query="/path with spaces/to your/bookmarks"

None of the options that are currently supported are likely to have double quotes in them, except potentially URLs, which are likely to have the encoding instead, so I don't think there would be a big issue with breaking existing configs. I personally think it's better to solve this by escaping double quotes like "option with a \" in it" than to escape spaces.

The vim script way seems to lean more towards escaping spaces like escaped\ space, but I and many other "vim" users are actually using Neovim and writing our configs in lua now, so we would use strings as arguments for functions instead. Most vim users also use the shell and will be familiar with quoting multi-word shell options, but most non-programmer/non-vim-user users of Vimium will not be at all familiar with the concept of escape sequences, so I would argue quoting multi-word options is the user-friendly compromise between vim users and the public.

The benefit of using something like %20 is that we don't even need to use a find and replace, we can get all the encodings for free just using decodeURI().

At first glance, it seems like we could just ignore spaces until we reach an "optionName=" but since we support unnamed options like reload hard and setZoom 1.2, we cannot do that. However, we can sidestep the issue the same way these two commands do if we don't want to come up with an implementation right now. As long as the command (activateBookmarks) doesn't have any other options it can take, with the current implementation it is easy to hijack the system and just use the full options array joined together as the path. So without any new functionality, we could implement this command like:

map asdf Vomnibar.activateBookmarks query=/path with spaces/to your/bookmarks

This would make the options list become ["/path", "with", "spaces/to", "your/bookmarks"], so you could just write optionList.join(" ") to get the full path back.

Again, I personally prefer the "quote multi-word options" design, but there are other options.

[UncleSnail]

Also, if you do decide to go with quoting out multi-word options, I'd be willing to write the parser changes if you would like.