update to nom 4

[Geal]

Hello,

I'm currently testing the upgrade to nom 4 on various crates, to see if I forgot anything. Currently, I get this error after doing the following changes:

failures:

---- test_time_with_timezone stdout ----
        thread 'test_time_with_timezone' panicked at 'assertion failed: `(left == right)`
  left: `Ok((CompleteByteSlice([43, 48, 53, 58]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 0, tz_offset_minutes: 0 }))`,
 right: `Ok((CompleteByteSlice([58]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 5, tz_offset_minutes: 0 }))`', tests/lib.rs:161:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

failures:
    test_time_with_timezone

test result: FAILED. 24 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

The most notable change is the introduction of the CompleteByteSlice type, which removes all Incomplete usage, assuming the input is complete. See the upgrade doc for more information.

[sylvestre]

@Geal @ignatenkobrain in the context of packaging exa in Debian, it would make our life easier to have this packaging using nom 4. Is that something you could do? Thanks

[badboy]

exa is using this? Wuih, I didn't know. I'm happy to merge nom 4 support and take on new maintainers, as I personally don't have much time to properly maintain it.

[ignatenkobrain]

FYI, in Fedora we just ship nom3 and do not care.

[hoodie]

of course exa is using it :smile: I basically pushed this for exa :grin:

[sylvestre]

@badboy could you please share an update on this? we are trying to get exa in Debian & Ubuntu and we would prefer to use nom v4. Thanks

[badboy]

What I said before stands. I don't have the time to maintain it. If you require it to work on nom v4 I'd be happy to merge support for it, but at least this PR is currently leading to test errors. If you submit a new one, I can spend some time on review and then also give you commit permission on this repository.

[infinity0]

If this doesn't work with nom-4 we are unlikely to be able to get exa into Debian Testing (freeze in Jan) because nom-3 has a whole dependency tree that we haven't yet packaged, which would be necessary for it to go in Debian Testing.

[Geal]

There's probably not much to do to fix that PR, but like @badboy I don't have time to spare for this. @infinity0 can someone from exa look into it?

[kpcyrd]

I'm somewhat confused by the test:

We have this case failing:

assert_eq!(Ok((Input(&b"+05:"[..]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 0, tz_offset_minutes: 0, })), parse_time(Input(b"16:43:16+05:")));

This is because we expect +05: to be left over, instead we get :. This is because both +05:00 and +05 are considered a valid timezone format. Changing this to only accept +05:00 causes a different test to fail that explicitly tests for +05.

assert_eq!(parse_datetime(Input(b"2009-02-01T09:00:22+05")),     Ok((Input(&[][..]), DateTime{ date: Date::YMD    { year: 2009,  month:02,  day:01},  time: Time{ hour: 9,   minute:0,   second:22,  millisecond: 0, tz_offset_hours: 5, tz_offset_minutes: 0}}))); 

Given noms nature I would argue that changing the test would be a reasonable solution. A user of this library would still need to check for trailing garbage anyway.

diff --git a/Cargo.toml b/Cargo.toml
index 69cc564..6470c3f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -13,8 +13,7 @@ readme = "README.md"

 [dependencies]
-nom = "4.0.0-beta1"
-clippy = {version = ">0.0.0", optional = true}
+nom = "4.0.0"

 [features]
 default = []
diff --git a/tests/lib.rs b/tests/lib.rs
index 39919e5..b6c1435 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -9,6 +9,7 @@ use nom::types::CompleteByteSlice;

 // alias the type for readability
 type Input<'a> = CompleteByteSlice<'a>;
+#[allow(non_snake_case)]
 pub fn Input<'a>(input:&'a[u8]) -> Input<'a> {
   CompleteByteSlice(input)
 }
@@ -158,9 +159,9 @@ fn test_time_with_timezone() {
     assert_eq!(Ok((Input(&[][..]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 5, tz_offset_minutes: 0, })), parse_time(Input(b"16:43:16+05:00")));
     assert_eq!(Ok((Input(&b"+"[..]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 0, tz_offset_minutes: 0, })), parse_time(Input(b"16:43:16+")));
     assert_eq!(Ok((Input(&b"+0"[..]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 0, tz_offset_minutes: 0, })), parse_time(Input(b"16:43:16+0")));
-    assert_eq!(Ok((Input(&b"+05:"[..]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 0, tz_offset_minutes: 0, })), parse_time(Input(b"16:43:16+05:")));
+    assert_eq!(Ok((Input(&b":"[..]), Time { hour: 16, minute: 43, second: 16, millisecond: 0, tz_offset_hours: 5, tz_offset_minutes: 0, })), parse_time(Input(b"16:43:16+05:")));

-    assert!(parse_time(Input(b"20:")).unwrap_err().is_incomplete());
+    assert!(parse_time(Input(b"20:")).is_err());
     assert!(parse_time(Input(b"20p42p16")).is_err());
     assert!(parse_time(Input(b"pppp")).is_err());
 }

[badboy]

I'm fine with adjusting tests if needed. I never really decided whether it should always parse the full line and thus fail with trailing stuff or just ignore that and let the user handle it.

We should at least document the behaviour.

[kpcyrd]

Awesome, I've submitted my patch as #21