Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make basename optional #280

Closed
blueyed opened this issue Jan 16, 2018 · 7 comments
Closed

Make basename optional #280

blueyed opened this issue Jan 16, 2018 · 7 comments

Comments

@blueyed
Copy link
Contributor

blueyed commented Jan 16, 2018

I prefer not having basename called inside the hunk - since this is where the line number is appended, and therefore is good for copy'n'pasting into your editor.

Currently there is foo:42, but the original would be bar/foo:42.

diff --git i/diff-so-fancy w/diff-so-fancy
index bece8e0..cd1397a 100755
--- i/diff-so-fancy
+++ w/diff-so-fancy
@@ -241,7 +241,7 @@ sub do_dsf_stuff {
                        }
 
                        my ($orig_offset, $orig_count, $new_offset, $new_count) = parse_hunk_
-                       $last_file_seen = basename($last_file_seen);
+                       # $last_file_seen = basename($last_file_seen);
 
                        # Figure out the start line
                        my $start_line = start_line_calc($new_offset,$new_count);

Related discussion: #100 (comment)

@scottchiefbaker
Copy link
Contributor

Can you give me a real world example of where/how this would change the d-s-f output?

@blueyed
Copy link
Contributor Author

blueyed commented Jan 17, 2018

Any diff where the file is in a subdirectory.

@scottchiefbaker
Copy link
Contributor

Ahhh I see... Honestly I don't know why we did it this way at all. It probably should just be the full path in the hunk line.

Should we make this the default? Or make it an optional setting?

@blueyed
Copy link
Contributor Author

blueyed commented Jan 17, 2018

I vote for making it the default.
/cc @paulirish

@scottchiefbaker
Copy link
Contributor

For what it's worth, I think my vote is making it default as well.

@paulirish
Copy link
Member

yeah seems good. let's do the full path.

scottchiefbaker added a commit to scottchiefbaker/diff-so-fancy that referenced this issue Jan 18, 2018
@scottchiefbaker
Copy link
Contributor

Ok this should be fixed in 49969f0. If there is anything else on this let me know and I'll reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants