Replace whole regexp dependency with custom function

[rumatoest]

Hello there. You know adding more dependencies is not a good idea especially for low level libraries. I think that in your project regexp dependency (with its transition dependencies) is a little bit redundant, because it required only once in a code.

Now in --build release you have only 4.8M dependencies in target/release/deps/ With regexp it was about 13M

[nastevens]

Totally appreciate what you're trying to do here. But one thing to point out is that checking the size of target/release/deps is not an accurate way to determine how much space a dependency takes in the final binary because often only a small portion of the dependency will actually be included in the binary. And I'm not worried about using an extra 8M on my development machine.

Rust is smart enough to remove code that isn't ever used. So in this case, the only time that the regex crate functions would be included in the final binary is if the library uses Pin::from_path. We can prove this using the cargo-bloat tool on the poll example:

>>> cargo bloat --example poll --release --crates

 File  .text     Size Name
33.2%  64.7% 183.8KiB std
17.3%  33.7%  95.8KiB [Unknown]
 0.8%   1.6%   4.5KiB sysfs_gpio
51.3% 100.0% 284.1KiB .text section size, the file size is 553.9KiB

The regex crate isn't included at all. Now if I edit the poll example to replace Pin::new with Pin::from_path:

>>> cargo bloat --example poll_from_path --release --crates

 File  .text     Size Name
20.8%  38.5% 234.9KiB std
12.7%  23.5% 143.3KiB regex
10.7%  19.7% 120.1KiB regex_syntax
 8.8%  16.3%  99.3KiB [Unknown]
 0.4%   0.8%   4.7KiB sysfs_gpio
 0.2%   0.4%   2.7KiB thread_local
 0.2%   0.4%   2.1KiB utf8_ranges
 0.1%   0.3%   1.6KiB memchr
 0.1%   0.1%     848B aho_corasick
54.1% 100.0% 609.5KiB .text section size, the file size is 1.1MiB

Now regex is included, and you're 100% right that it is the largest dependency. But it is adding 325kB, not 8MB. I find the new code less readable than a straight-forward regex, and I'm not convinced that the decrease in readability is worth 325kB. What do you think?

[rumatoest]

is not an accurate way to determine how much space a dependency takes

Agreed

I'm not convinced that the decrease in readability is worth 325kB. What do you think?

It does matter because it is 53% of whole file size, more than a half :)

As for readability. This is private API - normal dev would not see it, plus I've try to make understandable documentation for it.

Removing dependency is also about avoiding dependencies hell in future. What if your library user include another regexp (or it's transitive) dependency version which is not compatible with version you are using? Or maybe even some end user library would require such version of regexp or whatever. In Java world fixing such issues is a nightmare :(

[posborne]

I'm fine with removing the dependency but this code seems like it could be simplified. Although it was indirectly handled by the previous code, I see no reason to handle paths other than /sys/class/gpio/... -- that is, the wildcard in the middle of the path is not really necessary.

With that in mind, you can simplify things significantly by doing something like this:

let mut split = path.split("/");
match (split.next(), split.next(), split.next(), split.next()) {
    (Some("sys"), Some("class"), Some("gpio"), Some(gpio_name)) if gpio_name.starts_with("gpio") => {
        gpio_name.split_at(4).1
    },
    _ => {
        return Err(Error::InvalidPath(format!("{:?}", path)));
    }
}

In fact, you could probably just check to see if it starts_with "/sys/class/gpio/gpio" and split directly as well. There are also some formatting issues. Make sure to run the parts you have changed through rustfmt.

[rumatoest]

it was indirectly handled by the previous code

Indeed, patch became simpler now.

[posborne]

Change looks reasonable now. Can you squash these commits. There also appears to be a typo in your second commit message which should be fixed as well.

[nastevens]

Also, could you please split the rustfmt commit from the changes to replace regex? It makes it easier to review human-made changes from machine-made changes. Thanks!

[rumatoest]

I've simplified this patch

[nastevens]

Getting there! I made a couple suggestions to make things a little more idiomatic. Thanks for sticking with this!

[rumatoest]

And .... have you received an email when I made an update?

BTW if all your tests for rust 1.15 and OS X fails, why you do not remove those tests from CI?

[posborne]

Included (rebased) as part of #43. Thanks for the contribution!