sd icon indicating copy to clipboard operation
sd copied to clipboard

add --in-place flag

Open tennox opened this issue 2 years ago • 7 comments

See #191 for details

I gave it a shot. Tell me what you think :)

I've also parameterized the tests so the flag is also tested for the same cases.

I did some hyperfine benchmarks to make sure performance isn't severely degraded and it's mostly the same:

$ hyperfine -N -w1 -p 'cp source.txt test.txt' "/home/manu/dev/stuff/sd/target/debug/sd foo bar test.txt" "/home/manu/dev/stuff/sd/target/debug/sd --no-swap foo bar test.txt"

(tried with input sizes: 10B, 1KB, 10MB and containing foo on each line)

tennox avatar May 17 '23 09:05 tennox

As a heads up I'll be putting this on the back burner until after the v0.8.0 release. I'm focusing on getting existing functionality fixed up while saving new features for later

CosmicHorrorDev avatar May 17 '23 22:05 CosmicHorrorDev

LGTM

PeterlitsZo avatar May 28 '23 16:05 PeterlitsZo

(Just a random user's opinion, don't take this with authority)

I think taking the naming from sed would make this more easily discoverable, and is more easily understandable IMO:

-i[SUFFIX], --in-place[=SUFFIX]
    edit files in place (makes backup if SUFFIX supplied)

I'm not a fan of -i[SUFFIX] given -ijkl means -i -j -k -l, but -i=suffix could be nice.

ThinkChaos avatar Dec 08 '23 23:12 ThinkChaos

Merged latest master & reworked the implementation accordingly (which made it a good bit simpler :+1: )

tennox avatar May 10 '24 15:05 tennox

I think taking the naming from sed would make this more easily discoverable, and is more easily understandable IMO:

-i[SUFFIX], --in-place[=SUFFIX]
    edit files in place (makes backup if SUFFIX supplied)

@ThinkChaos thanks, I took (part of) your suggestion, and renamed to --in-place (no short flag / custom suffix implemented).

tennox avatar May 10 '24 16:05 tennox