| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Previously we'd get an error if the first lines in the mappings file
were blank or comments.
|
|
This broke recently I think. You can no longer have a mappings file that
starts with blank lines or comment lines.
Removing the empty and blank handler in `definitions()` fixes the tests
added by this commit but fails these:
parser::tests::map_group_empty_input_does_not_fail
parser::tests::map_group_skipped_input_outputs_default_map_group
Using this diff:
diff --git a/src/parser.rs b/src/parser.rs
index 3f3d7b9..d18b56a 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -629,11 +629,11 @@ where
I: Stream<Item = char>,
I::Error: ParseError<I::Item, I::Range, I::Position>,
{
- or(
- (
- blank(),
- eof(),
- ).map(|_| MapGroup::default()),
+ // or(
+ // (
+ // blank(),
+ // eof(),
+ // ).map(|_| MapGroup::default()),
(
definitions(),
eof(),
@@ -661,8 +661,8 @@ where
}
map_group
- }),
- )
+ })
+ // )
}
fn comment<I>() -> impl Parser<Input = I>
Need to figure out some way to get both sets of tests to pass.
|
|
Check that the desired error messages appear for a couple extra invalid
mapping definitions.
Rename the existing `map_group_shows_error_in_middle_of_line()` test to
use the naming pattern of the two new tests.
The new tests check that the right errors appear in map actions, and for
missing the closing brace on a mode definition.
|
|
Just noticed this. Seems it served its purpose.
|
|
In addition to asserting the position of the error, also check that it's
the error we expect by looking for an unexpected 'n' token.
|
|
This seems to fix the problem of errors in the middle of definitions not
being surfaced (what the test in
3091ebe0deb1ba832b4a5925263409ec5b651c62 is for).
Need to write some more tests for other error cases.
The reason why this happens is because `try()` causes the parser to not
consume the input. Since the input isn't consumed, I guess the error
messages from within never get surfaced. Moving the `try()` further down
the parser tree appears to fix things.
|
|
I wrote the wrong column number on which the error should occur.
|
|
Since the errors are going to be different from what's written in the
test expectation, just remove them to correct the test. We kind of only
care about the position. Maybe the first error would be interesting to
match on, but let's just get rid of the part we know is wrong.
|
|
|
|
|
|
This functions correctly. No need for the TODO.
|
|
We currently correctly report errors in the middle of the first line
(problems with the trigger, problems with special keys in an action),
but for all subsequent lines, the error message is limited to the first
character on the line. This means that even if you write "map", "cmd",
or "mode", which are all correct, the error message will refer to
"column 1", even though the actual error is in a different part of the
parser.
Not sure why that's happening, nor why it doesn't happen for the first
line. But at least we have a test for it now. The test will need to be
modified once I can actually get the error messages propagating
correctly, as the `Expected` lines are going to be wrong. They were just
copy-pasted from the test above this one.
|
|
This was split into two new functions, `map_kind_map()` and
`map_kind_cmd()`, in 7776832ec11ee7d4e62cfd2a6ad7735f323ab5bc. As such,
this function is no longer used or needed.
Update the tests to use the new parser functions.
|
|
Revert changes from 75d52e385fa66d6e151c9baa2cf22c2223c39ff0. These are
obsolete.
Now that we parse actions right in the main parser
(7776832ec11ee7d4e62cfd2a6ad7735f323ab5bc), get rid of all code related
to the second pass parser from `Action::String`s into `Action::Map`s, as
it's obsolete.
All of this action parsing is now handled by the main `map_group()`
parser.
|
|
Was having a hell of a time trying to return a `Result` from
`MapAction::parse()` in 75d52e385fa66d6e151c9baa2cf22c2223c39ff0. After
struggling with it, I started to think if there was a different way to
go about this. Previously I had tried and failed to parse map actions in
the main parser. If I could get that working, then this whole
`MapAction::parse()` method would be obsolete. No need to muss with
`Result`s and borrowed strings at all.
When I originally wrote the parser code, I thought in terms of: "How do
I get the parsed result of the `map()` parser into the `action()` parser
so that it can conditionally parse one way for the `map` keyword, and
another for the `cmd` keyword?". In hindsight, this was incredibly
idiotic, but I guess I was too close to the code and too deep in the
details.
Obviously the answer to parsing differently depending on `map` or `cmd`
is not to get the parsed result of `map()` into `action()`. These are
parser combinators. _Of course_ we can just make two map definition
parsers, one for `map` and another for `cmd`. Can't believe I never
thought of that last time. Who knows how much time I wasted trying and
failing, then writing a sub-optimal solution.
Oh well, I guess I should just be thankful that a niggling logging
problem (in `MapAction::parse()`) that turned into a maddening journey
to a giant brick wall in search of a `Result` return value with correct
borrows finally enlightened me to the fact that I was doing the wrong
thing all along.
* Comment out double-parsing code related to `MapAction::parse()`
because we want to parse in a single go. No more two-pass parsing.
* Split the `map_kind()` parser into two separate parsers for `map` and
`cmd`. Similarly, split the `map()` parser into `map_map()` and
`map_cmd()` for the same reason. Our new parser tree looks like this:
map(): map definition line
-> map_map(): `map` definitions
-> map_kind_map(): `map`
-> map_cmd(): `cmd` definitions
-> map_kind_cmd(): `cmd`
* Up the `recursion_limit` to 256 from 128 because I was getting stack
size errors randomly when I was updating the tests for this code
change.
* Update `action_character()` to not parse `\n` newlines. Otherwise the
newline gets included as a real `Character` in the
`KeyboardKeyWithModifiers` vector.
* The `map_collection_fails_without_terminating_newline()` test fails as
a result of the `action_character()` parser change. Previously it
relied on `map` lines parsing to `Action::String`s, which fail if
there's no terminating newline. Now that `map`s parse to
`Action::Map`s, the newline isn't checked by the map line parser
(`map_map()`). Because the newline doesn't matter for `map`
definitions, the parser would return `Ok`, failing this test even
though the behaviour was correct. Change the map line to a `cmd` to
have the test check the behaviour it was previously testing.
* Update the tests to use parsed `Action::Map` instead of
`Action::String`s where appropriate. Used this program to convert the
strings into the correct source code:
# action_string_to_action_map.py
#!/usr/bin/env python3
import sys
action_string = sys.argv[1]
action = ''
action += 'action: Action::Map(vec![\n'
for c in action_string:
action += ' KeyboardKeyWithModifiers::new(\n'
action += " KeyboardKey::Character(Character::new('{}')),\n".format(c)
action += ' vec![],\n'
action += ' ),\n'
action += ']),'
print(action)
|
|
Want to be able to return a `Result` here so we can print the error from
the `ffi` module so we don't have to permit 'stderrlog' on this parser
module.
Beset with errors like this:
error[E0506]: cannot assign to `self.action` because it is borrowed
--> src/parser.rs:188:21
|
176 | Action::String(ref s) => {
| ----- borrow of `self.action` occurs here
...
188 | self.action = action;
| ^^^^^^^^^^^^^^^^^^^^ assignment to borrowed `self.action` occurs here
error: aborting due to previous error
Even cloning the `Action` doesn't work because the Combine error
`Result` needs an `&str` reference to the parser input, and if we
reassign `self.action`, that string reference would disappear.
Need to figure out a different way of dealing with this, but I at least
want to commit what I have because the next step is going to be
something different.
|
|
Previously, the modified input in `map_group_with_invalid_input_fails()`
in this commit would fail with a result of `Ok` instead of `Err`. The
`map_group()` parser would parse the first, correct line, and ignore the
second, incorrect one.
That's wrong. We want the whole parser to fail because it contains an
invalid sequence. Do this by ensuring `eof()` follows all
`definitions()`. This way we're guaranteed to always parse the whole
input, and any errors within get surfaced.
|
|
This type is already imported. No need to use the full path.
|
|
In 13e90c090923a209e5e26fb3e609d5d12f737f53 I decided to make `Map` a
type alias to a tuple containing the key-value pair used by
`MapCollection`.
Here I've decided that I prefer the type as it was before. Sure, it was
more of an intermediary type, but it did have a name that was used
consistently. We do have to leave in our custom `HashMap` building, so
perhaps this isn't the ideal solution, but I'm going with it, at least
for now.
|
|
An input with all mixed whitespace and comments should behave the same
as empty input.
|
|
Since the most recent `many1` change
(09c0a432fb339a2218096bb9a4398fb86301488f), the parser fails on
end-of-input, or an empty string.
In that case, it should instead return the default `MapGroup`. Add a
special case for this.
|
|
Thanks to Arnavion on Mozilla#rust for clueing me into this:
> The documentation starts with "A parser in this library can be
> described as a function which takes some input and if it is
> successful, returns a value together with the remaining input."
> Key phrase being "with the remaining input"
> So I assume it just matches as best as it can (which is nothing, in
> your case)
> Maybe you want to use some one-or-more combinator rather than many
> which is a zero-or-more combinator
> Then add your own check that "the remaining input" is empty
Replacing my `many()` parsers with `many1()` correctly propagates errors
up from sub-parers. Nice!
This will fail when we try to parse an empty string, but we can just add
a special case for that to parse successfully.
|
|
Investigating why I'm not getting error messages in my parsed result.
The layer where I stop getting them is when parsing to a
`MapCollection`.
Split the list of `map()` parser out from `map_collection()` to get more
information. Turns out that the new `maps()` parser is now where parse
errors get discarded.
In an attempt to get the errors to appear, I tried to replicate the
structure of Combine's INI example parser:
https://github.com/Marwes/combine/blob/921202a018000041c9d3e8b12b7c1a53d0252f67/examples/ini.rs
That program makes a `property()` parser which outputs a 2-tuple of
`(String, String)`, and a `properties()` parser which outputs a
`HashMap<String, String>`, using the values in the tuple to construct
the HashMap.
Unfortunately, this change didn't solve the problem of the non-bubbling
error messages. Unsure about whether or not to keep this change.
|
|
This was here for reference. No longer needed.
|
|
This means that if any of `<Up>`, `<Play>`, or `<Down>` are undefined in
the mappings file definition, they will be set to their default action
values (as set in `MapGroup::default()`).
To get rid of the default, map the button trigger to `<Nop>`.
Update a test that didn't map `<Up>` to give it the default mapping.
|
|
If a mappings file is found but is empty, no mappings will be set. Hmm,
that sounds wrong. We still want to keep the defaults even if only some
button triggers are mapped. Making buttons do nothing is what `<Nop>` is
for, not this behaviour. Darn.
|
|
This idea is replaced by code from 'autopilot', which lives in the
`autopilot_internal` module.
|
|
Seemed to make more sense as it will be printed to the daemon's stderr
log when reloading mappings.
|
|
Don't panic on error. Instead display the error to the user.
|
|
Looking at the function, it makes more sense to put it inside
a `MapAction` impl since it operates on a `MapAction`.
|
|
I had written that TODO when I thought we were going to be parsing
`Action::Command`s. I ended up completely getting rid of
`Action::Command`, and putting commands in `Action::String`s instead.
|
|
|
|
Get rid of these warnings:
warning: private type `parser::Character` in public interface (error E0446)
--> src/parser.rs:72:15
|
72 | Character(Character),
| ^^^^^^^^^
|
= note: #[warn(private_in_public)] on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>
warning: private type `parser::KeyCode` in public interface (error E0446)
--> src/parser.rs:73:13
|
73 | KeyCode(KeyCode),
| ^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>
|
|
I wasn't using it. At first I added it because it made sense, but
ultimately I ended up using `Action::String` for commands instead
because that was easier.
|
|
This function isn't used
|
|
|
|
This "key" creates an empty action that doesn't do anything. Allows
users to clear the default behaviour of a headphone button.
|
|
|
|
Add a new parser for `NXKey`s. I originally thought I was going to add
this to the `key_code()` parser, but that one parses to `KeyCode`s.
Seemed like it would be easier to make a brand new parser to handle the
different type.
Need to add tests for this.
|
|
Add this new key type to the enum and enable it to be `tap()`ped like
the others.
Made `NXKey` a type alias instead of a struct to allow us to refer to
any of the NX key constants using the `NXKey` type. This also meant
moving those NX constants outside of the `NXKey` impl and into the top
level. May end up wrapping them back up in some kind of namespace later.
Adjusted the public-ness of some functions to support this change.
|
|
Commands now stay `Action::String`s without getting re-parsed to
`Action::Command`s (as per 79dfb6c8cd536e9448f05421d5018785a8b590ce).
This test is therefore unnecessary.
|
|
Previously I had only implemented it for top-level maps. Get it working
for in-mode maps too. Move the parsing code to a function so it can be
re-used in both places.
|
|
Non-working attempts.
|
|
Use `autopilot::key::tap()` to simulate each key and modifier in an
action.
Originally tried to do this using a method that takes a closure to
access the `KeyCodeConvertible` types inside `KeyboardKey`. Ended up
with a much cleaner API solution that only requires a single method
call.
|
|
I originally wrapped the contained `Vec` in an `Option` to that we
wouldn't have to initialise whole new empty `Vec` for every key in an
`Action` list.
However, this made less sense once I wrote the parser and ended up with
an empty `Vec` by default that I then had to convert into an `Option`.
Now I've started the code to type keys including modifiers, and the
`Option` no longer makes any sense at all. In order to type keys
(including modifiers), I'll be using Autopilot's `tap()` function:
pub fn tap<T: KeyCodeConvertible + Copy>(key: T, flags: &[Flag], delay_ms: u64) {
This function requires a slice, so I ended up getting a `Vec` from my
parser, converting it to an Option to satisfy my original interface, and
now would have to convert it back into a `Vec` or slice in order to be
able to pass it into `tap()`. Talk about unnecessary.
Get rid of the `Option` because it's more work for absolutely no reason.
|
|
Get rid of old commented code now that we have a working version.
|
|
First, `MapGroup::parse` parses actions to `Action::String`s. We then
run a second pass on the parsed actions to parse them into
`Action::Map`s.
A few failed attempts but finally got it working. Tried a few ways of
doing it and kept running into various borrowing problems. Glad it's
working now.
|
|
This reverts commit 5ef2443642a2d8b223afdf169200a725d2809b76. See that
commit for details.
|
|
Trying to add new escape special keys for '\' and '<' but having a bit
of trouble.
Looks like I'm not using `and` `satisfy` correctly, as I'm getting this
error:
error[E0271]: type mismatch resolving `<(impl combine::Parser, combine::combinator::Satisfy<_, [closure@src/parser.rs:263:56: 263:68]>) as combine::Parser>::Output == char`
--> src/parser.rs:260:18
|
260 | (choice!(
| __________________^
261 | | try(string_case_insensitive("Bslash")).map(|_| '\\'),
262 | | try(string_case_insensitive("lt")).map(|_| '<'),
263 | | try(action_character().and(satisfy(|c| c != '>')))
264 | | ).map(|c|
| |_________________^ expected tuple, found char
|
= note: expected type `(char, _)`
found type `char`
= note: required because of the requirements on the impl of `combine::Parser` for `combine::combinator::Try<(impl combine::Parser, combine::combinator::Satisfy<_, [closure@src/parser.rs:263:56: 263:68]>)>`
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
I had added the `satisfy` not '>' after getting this test failure
without it:
---- parser::tests::action_parses_map_with_bslash_and_lt_special_keys stdout ----
thread 'parser::tests::action_parses_map_with_bslash_and_lt_special_keys' panicked at 'assertion failed: `(left == right)`
left: `Err(Errors { position: PointerOffset(4332007031), errors: [Unexpected(Token('l')), Expected(Token('>'))] })`,
right: `Ok(Map([KeyboardKeyWithModifiers { key: Character(Character(Character('a'))), flags: None }, KeyboardKeyWithModifiers { key: Character(Character(Character('\\'))), flags: None }, KeyboardKeyWithModifiers { key: Character(Character(Character('A'))), flags: None }, KeyboardKeyWithModifiers { key: Character(Character(Character('N'))), flags: None }, KeyboardKeyWithModifiers { key: Character(Character(Character('D'))), flags: None }, KeyboardKeyWithModifiers { key: Character(Character(Character('<'))), flags: None }, KeyboardKeyWithModifiers { key: Character(Character(Character('>'))), flags: None }, KeyboardKeyWithModifiers { key: Character(Character(Character('\\'))), flags: Some([Control]) }, KeyboardKeyWithModifiers { key: Character(Character(Character('<'))), flags: Some([Meta, Shift]) }]))`', src/parser.rs:928:9
as I suspected this was a problem with `<lt>`. But now that I think
about it, it could just as easily have been a problem with `<Bslash>`.
Not sure.
Anyway, I'm thinking of dropping these escapes because they're redundant
(we already have '\' escaping) and because I'm tired of these errors.
|
