Clean up the headers to indicate a file was added/removed/modified

[scottchiefbaker]

Also makes the file name "clickable" by removing the leading a/ and /b

[scottchiefbaker]

My sed skills are seriously lacking, but Perl skills are decent. I went ahead and took a stab at addressing #28 by calling a Perl script I put in /third_party/

Let me know if I'm going down the wrong path here.

[paulirish]

i believe this would qualify as first-party code, unless you're releasing this separately. :)

I hesitate a bit to 👍 adding perl to our sourcebase. I'm certainly way less comfortable in it than bash. Would you consider porting your work here to bash?

[paulirish]

curious what others think, too. I'm flexible if the consensus leans toward the perl implementation.

[stevemao]

I'm concerned with how we can handle one extra dependency... (I miss nodejs 😄) We need to update the install section in README and might need to deal with related issues, etc.

[noscript]

diff-highlight is a Perl script, so shouldn't be an issue.

[paulirish]

yeah install-wise it wont be a problem, so that's an upside.

@scottchiefbaker wanna write a few tests?

[noscript]

As diff-so-fancy gets more complicated, I would rather rewrite it entirely in Perl. Which would also make it faster.

[scottchiefbaker]

I don't know enough bash to make this sort of thing. Someone with more bash skills wouldn't have too much of a problem, the script is pretty simple.

If we're comfortable with a Perl implementation I can take a look at adding tests. Otherwise I'm happy to work with someone who wants to write a bash implementation, to explain my logic.

[paulirish]

yeah i am feeling the pain of bash as well. i'm into porting everything into perl.
(better yet, javascript, but that's a dependency that may be too much to ask)

[scottchiefbaker]

I know enough Perl that I can contribute more if we go down that road. What's the next/first step to going down that road?

Do we want to land this change for clickable headers and start a separate issue for migrating some of the other pieces to Perl?

[paulirish]

As diff-so-fancy gets more complicated, I would rather rewrite it entirely in Perl. Which would also make it faster.

I was curious about this so I ran some numbers.

perf of diff-so-fancy

I took a fairly busy diff of ~10 files as my example. I broke apart the 4 pieces of the pipeline, and ran them a few times. These are the rough medians:

      git diff: █████ 50ms
diff-highlight: ██████████ 100ms
 diff-so-fancy: █████████████ 130ms
  header_clean: ███ 30ms

(I was running them with time bash -c "…" so there's ~10-20ms of overhead built into the numbers.)

Pretty interesting. Bash is certainly not giving us speed here. :)

[noscript]

Multiple calls to sed is the major slow down. We could make the current version faster if do all in one call, but the readability will suffer. Not worth the effort.

[scottchiefbaker]

I count 9 calls to sed in diff-so-fancy. At least four of them (in format_diff_header) could easily be combined.

[paulirish]

I'm +1 for merging this.

I'm also interested in migrating the rest of the script into perl, as I think maintenance and readability would improve.

@scottchiefbaker can you move the script to a new ./lib folder?

Also. a test? :)

[scottchiefbaker]

Yes I can move this to a .libs folder.

I looked at adding tests, and I can't even get the existing tests to run locally. No matter what I do I get:

bats: /home/bakers/github/diff-so-fancy/test/test_helper/bats-core/load.bash does not exist

Any tips on setting up a testing environ so I can start writing tests?

[paulirish]

@scottchiefbaker did you git submodule update --init ?

[scottchiefbaker]

I cleaned up my pull request and added tests for the functionality I added. This should address all the outstanding issues/questions, and should be ready to merge.

[paulirish]

lookin sharp. merging…

[stevemao]

@scottchiefbaker can you have a look at Homebrew/legacy-homebrew#49712? Thanks.

[scottchiefbaker]

Checking it out now...