rust-cli / rexpect

.github/workflows/ci.yml
https://docs.rs/rexpect
MIT License
328 stars 56 forks source link

make match_any also return index of matched needle #22

Closed philippkeller closed 4 years ago

philippkeller commented 4 years ago

Public api stays the same except in exp_any which is additionally returning an index.

I'm not 100% happy with the change as I needed to add an additional function read_until_tuple_pos which serves as a common code base between the any-case and the other cases. This returns also the index which is 0 for the non-any cases.

But I couldn't come up with anything better. This is a breaking change so I bumped up the version.

This solves #14.

What do you think, @pbartyik, would this solve your use case? Maybe @zhiburt, as you're most probably more into Rust than me at the moment, if you find anything which could be improved, then I'd be glad for input

zhiburt commented 4 years ago

Hi @philippkeller, I suppose the #14 issue is actually a reasonable one.

It definitely caused my interest and a spend a little time on the draft of a possible design. And first of all I really don't know the cost of this. And now I even think might I was involved not in the issue I should have (edited: definitely :( ) as we have this 3 parameters :) but that what I've got so far.

We could split the logic of ReadUntil to different types which united by Needle trait.

pub struct Match<T> {
    begin: usize,
    end: usize,
    interest: T,
}

pub trait Needle {
    type Interest;

    fn find(&self, buffer: &str, eof: bool) -> Option<Match<Self::Interest>>;
}

In this case each implementation of Needle would decide what parameters it should return.

read_until could look like that

    pub fn read_until<N: Needle + ?Sized>(&mut self, needle: &N)
       -> Result<(String, String, N::Interest)>

with such changes


            if let Some(m) = needle.find(&self.buffer, self.eof) {
                let first = ...;
                let second = ...;
                return Ok((first, second, m.interest));
            }

All that could safe the old interface or use a generic approach like that.

    fn test() {
        || -> Result<()> {
            let mut p = spawn("cat", Some(1000)).expect("cannot run cat");
            p.send_line("hello world!")?;
            p.exp("hello world!")?;
            p.send_line("hello heaven!")?;
            let arr = vec![reader::ReadUntil::EOF, ReadUntil::NBytes(10)];
            let (_, _, position) = p.exp(&arr[..])?;
            assert_eq!(position, 1);
            Ok(())
        }()
        .unwrap_or_else(|e| panic!("test_expect_string failed: {}", e));
    }

The 100% downside is the third parameter in most cases is () which is a pity

In any way the draft is passed all tests.

philippkeller commented 4 years ago

closing this in favor of #24 which solves this much nicer