Use new algorithm for line replacements

[dmerejkowsky]

Fix #15

[codecov-commenter]

Codecov Report

Merging #39 into master will increase coverage by 4.34%.
The diff coverage is 86.20%.

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
+ Coverage   74.55%   78.89%   +4.34%     
==========================================
  Files           8        7       -1     
  Lines         334      379      +45     
==========================================
+ Hits          249      299      +50     
+ Misses         85       80       -5     

[dmerejkowsky]

TODO: somehow the original error is lost when opening or writing a file fails. Needs more investigation

[cgestes]

Cool :)

Seems pretty hard to review though...

[dmerejkowsky]

Yeah - there's lots of stuff going on. I'll open smaller pull requests to make review easier.

[dmerejkowsky]

@cgestes take a look at #42 please :)

[cgestes]

Sorry. Holliday's. Will check :)

[dmerejkowsky]

Note for reviewers: I'm finally happy with the shape of the code, tell me if the diff is too hard to review and I try to split in more commits

[blastrock]

Is there really no way to factorize this loop? Here's my intuition:

// put this in print_self:
let line = self.apply(input, |old, _new| old.red().underline());
println!(line);

// with:
    pub fn apply(&self, input: &str, transform: &Fn<(old:str, new:str) -> String>) -> String {
        let chars: Vec<_> = input.chars().collect();
        let mut i = 0;
        let mut res = String::new();
        while i < input.len() {
            if let Some(ReplaceValue { end_pos, new, .. }) = self.values.get(&i) {
                let old = &input[i..*end_pos];
                res.push_str(transform(old,new)); // <<<< called here
                i = *end_pos;
            } else {
                let c = chars[i];
                res.push(c);
                i += 1;
            }
        }
        res
    }

Please forgive my rust, it might not compile and express ideas that are against the compiler.

[dmerejkowsky]

I was looking for such a comment :) Did not thought about taking a closure as parameter, let me see if I can make it compile

[dmerejkowsky]

Also, the code crashes:

thread 'main' panicked at 'index out of bounds: the len is 132812 but the index is 132812', src/line_patcher.rs:55:25
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1076
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:217
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:526
  11: rust_begin_unwind
             at src/libstd/panicking.rs:437
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
  13: core::panicking::panic_bounds_check
             at src/libcore/panicking.rs:62
  14: <usize as core::slice::SliceIndex<[T]>>::index
             at /home/dmerej/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/slice/mod.rs:3103
  15: core::slice::<impl core::ops::index::Index<I> for [T]>::index
             at /home/dmerej/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/slice/mod.rs:2955
  16: <alloc::vec::Vec<T> as core::ops::index::Index<I>>::index
             at /home/dmerej/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/liballoc/vec.rs:1975
  17: ruplacer::line_patcher::Replacements::apply
             at src/line_patcher.rs:55
  18: ruplacer::line_patcher::patch_line
             at src/line_patcher.rs:25
  19: ruplacer::file_patcher::FilePatcher::new
             at src/file_patcher.rs:40
  20: ruplacer::directory_patcher::DirectoryPatcher::patch_file
             at src/directory_patcher.rs:43
  21: ruplacer::directory_patcher::DirectoryPatcher::run
             at src/directory_patcher.rs:31
  22: ruplacer::run_on_directory
             at src/main.rs:233
  23: ruplacer::main
             at src/main.rs:212
  24: std::rt::lang_start::{{closure}}
             at /home/dmerej/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67
  25: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  26: std::panicking::try::do_call
             at src/libstd/panicking.rs:348
  27: std::panicking::try
             at src/libstd/panicking.rs:325
  28: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  29: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  30: std::rt::lang_start
             at /home/dmerej/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67
  31: main
  32: __libc_start_main
  33: _start

Use cargo run -- CONFIG_URLS foo-bar with the attached js bundle to reproduce
bundle.zip

[dmerejkowsky]

Update from fifi:

in self.values we store positions that are taken from the &str, and we compare them with positions in the Vec<char> - something's wrong

[dmerejkowsky]

Also also the new algo does not work:

ruplacer 'from path import Path' 'from pathlib import Path'
-- from pathlib import Path
++ from pathlib import Pathath

[dmerejkowsky]

This is not going anywhere, let's close this