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

🐛 Why is there a line number in the hunk header when code line numbers are enabled (and more)? #494

Closed
erikhuizinga opened this issue Jan 7, 2021 · 4 comments

Comments

@erikhuizinga
Copy link

erikhuizinga commented Jan 7, 2021

Relevant Git config:

[core]
	pager = delta --diff-so-fancy

[color]
	ui = true

[interactive]
    diffFilter = delta --color-only

[delta]
    features = side-by-side line-numbers 
    syntax-theme = Dracula

When running git diff 0.5.0...0.5.1 on the delta repository, this is the output in my shell (raw Git diff below):

image

Raw Git diff

git --no-pager diff 0.5.0...0.5.1

(only the corresponding part)

diff --git a/Cargo.lock b/Cargo.lock
index 6dd42e4..9a77308 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1,16 +1,16 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
 [[package]]
-name = "adler32"
-version = "1.0.4"
+name = "adler"
+version = "0.2.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5d2e7343e7fc9de883d1b0341e0b13970f764c14101234857d2ddafa1cb1cac2"
+checksum = "ee2a4ec343196209d6594e19543ae87a39f96d5534d7174822a3ad825dd6ed7e"
 
 [[package]]
 name = "aho-corasick"
-version = "0.7.6"
+version = "0.7.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "58fb5e95d83b38284460a5fda7d6470aa0b8844d283a0b614b8535e880800d2d"
+checksum = "7404febffaa47dac81aa44dba71523c9d069b1bdc50a77db41195149e17f68e5"
 dependencies = [
  "memchr",
 ]
@@ -42,17 +42,11 @@ dependencies = [
  "winapi",
 ]
 
-[[package]]
-name = "arrayref"
-version = "0.3.5"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0d382e583f07208808f6b1249e60848879ba3543f57c32277bf52d69c2f0f0ee"
-
 [[package]]
 name = "arrayvec"
-version = "0.5.1"
+version = "0.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "cff77d8686867eceff3105329d4698d96c2391c176d5d03adc90c7389162b5b8"
+checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b"
 
 [[package]]
 name = "atty"

Note the 2 hunk headers:

  • The first contains line number 1 followed by a colon and then nothing.
  • The second contains line number 42 followed by a colon, a space and part of the code.
  • The first line after the hunk header shows the same line number and the contents of the line, which is not the same as the part of the code mentioned above.

This leads to my questions/issues:

  • Should there be a colon after line number 1, which is never preceded by any code?
  • Should there be a line number by default in the hunk header when line numbers in the code are enabled? The same line number is shown, which to me doesn't seem helpful in any way.
  • It is confusing (it's a Git thing, I know) to have a line of code (or none) in the hunk header right after a line number that do not correspond to each other! As a user, I expect delta to make my Git CLI a bit more helpful / human friendly. Showing a line number in the header is only helpful if there are no line numbers elsewhere.
  • How would one config delta to not show hunk header line numbers, but show part of any code preceding the hunk (this often contains useful context info)?

Here's another screenshot followed by the raw Git diff, this time using git diff 0.5.0...0.5.1 -- *.rs:

image

Raw Git diff

git --no-pager diff 0.5.0...0.5.1 -- *.rs

(only the corresponding part)

diff --git a/src/cli.rs b/src/cli.rs
index cd1555d..ee3baac 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -372,6 +372,18 @@ pub struct Opt {
     /// output.
     pub hunk_header_style: String,
 
+    #[structopt(long = "hunk-header-file-style", default_value = "blue")]
+    /// Style (foreground, background, attributes) for the file path part of the hunk-header. See
+    /// STYLES section. The file path will only be displayed if hunk-header-style contains the
+    /// 'file' special attribute.
+    pub hunk_header_file_style: String,
+
+    #[structopt(long = "hunk-header-line-number-style", default_value = "blue")]
+    /// Style (foreground, background, attributes) for the line number part of the hunk-header. See
+    /// STYLES section. The line number will only be displayed if hunk-header-style contains the
+    /// 'line-number' special attribute.
+    pub hunk_header_line_number_style: String,
+
     #[structopt(long = "hunk-header-decoration-style", default_value = "blue box")]
     /// Style (foreground, background, attributes) for the hunk-header decoration. See STYLES
     /// section. The style string should contain one of the special attributes 'box', 'ul'
diff --git a/src/config.rs b/src/config.rs
index 7331a12..d782895 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -31,6 +31,8 @@ pub struct Config {
     pub file_renamed_label: String,
     pub file_style: Style,
     pub git_config_entries: HashMap<String, GitConfigEntry>,
+    pub hunk_header_file_style: Style,
+    pub hunk_header_line_number_style: Style,
     pub hunk_header_style: Style,
     pub hunk_header_style_include_file_path: bool,
     pub hunk_header_style_include_line_number: bool,
@@ -107,8 +109,13 @@ impl From<cli::Opt> for Config {
             whitespace_error_style,
         ) = make_hunk_styles(&opt);
 
-        let (commit_style, file_style, hunk_header_style) =
-            make_commit_file_hunk_header_styles(&opt);
+        let (
+            commit_style,
+            file_style,
+            hunk_header_style,
+            hunk_header_file_style,
+            hunk_header_line_number_style,
+        ) = make_commit_file_hunk_header_styles(&opt);
 
         let (
             line_numbers_minus_style,
@dandavison
Copy link
Owner

Hi @erikhuizinga, thanks for this.

Short answer: to disable the line number in the hunk header (but retain the file path), use hunk-header-style = file syntax.

Should there be a line number by default in the hunk header when line numbers in the code are enabled? The same line number is shown, which to me doesn't seem helpful in any way.

That was my first thought also and in fact I initially implemented it that way. However, (a) the line number in the hunk header as /path/to/file.rs:77 is actually very useful even if it is duplicated in the line-numbers panel because, with certain setups such as iTerm2 semantic history, it is possible to click on that element in a terminal emulator and have your text editor/IDE open the file at the correct line number. See #473 (comment) and #481 (comment). And (b) the line number in the hunk header can be disabled using e.g. hunk-header-style = file syntax.

It is confusing (it's a Git thing, I know) to have a line of code (or none) in the hunk header right after a line number that do not correspond to each other! As a user, I expect delta to make my Git CLI a bit more helpful / human friendly.

Yes, as you say, it's a git thing: if we decide to retain the line number in the hunk header, we have to get used to thinking of it as "Your hunk starts at these coordinates. By the way, here is a line of code that I have identified as probably helping you see the indentation context (e.g. function, class) in which your hunk lies. But the coordinates are not of that line of code; it is somewhere above."

How would one config delta to not show hunk header line numbers, but show part of any code preceding the hunk (this often contains useful context info)?

hunk-header-style = file syntax or hunk-header-style = syntax

Should there be a colon after line number 1, which is never preceded by any code?

Ha, it's a bit of an edge case isn't it :) I think when I was writing the code I played around with conditional inclusion of the colon. But no, I think it's OK to keep the colon there for consistency, especially as tools such as iTerm2 semantic history have regexps recognizing the standard file.rs:<line>:<col> output format. If you have removed both line number and file path, e.g. via hunk-header-style = syntax, then that first hunk header will be omitted entirely.

The relevant section in --help is

--hunk-header-style <hunk-header-style>
    Style (foreground, background, attributes) for the hunk-header. See STYLES section. Special attributes
    'file' and 'line-number' can be used to include the file path, and number of first hunk line, in the hunk
    header. The style 'omit' can be used to remove the hunk header section from the output [default: line-
    number syntax]
--hunk-header-file-style <hunk-header-file-style>
    Style (foreground, background, attributes) for the file path part of the hunk-header. See STYLES section.
    The file path will only be displayed if hunk-header-style contains the 'file' special attribute [default:
    blue]
--hunk-header-line-number-style <hunk-header-line-number-style>
    Style (foreground, background, attributes) for the line number part of the hunk-header. See STYLES section.
    The line number will only be displayed if hunk-header-style contains the 'line-number' special attribute
    [default: blue]

@erikhuizinga
Copy link
Author

Thanks for you elaborate answer. It looks like I might have to look into a different shell app to try such clicking to editor/IDE features!

Closing as answered and already addressed/considered.

@dandavison
Copy link
Owner

dandavison commented Jan 7, 2021

No problem. Some possibilities that you might want to investigate are

  1. delta hyperlinks=true. For the hyperlinks to work you need a compliant terminal emulator AND the latest version of less. (And if you're using tmux, then a patched version of tmux.) I've written a tool to help create custom URL handlers to do things like open an arbitrary editor at a particular file and line: https://github.com/dandavison/open-in-editor
  2. Like I mentioned, if you're on MacOS then iTerm can out-of-the-box handle certain link formats and open in your editor. I also have an older project implementing arbitrary click handlers for iTerm2 by mapping regexes to editor-opening commands.

Err, I don't mean to plug random projects of mine, this just happens to be something I've played with. The hyperlinks spec seems promising for terminal emulators.

@erikhuizinga
Copy link
Author

I get it: you're a shell/terminal hacker and you like it. I like the tools that people like you produce, so no worries! Thanks for pointing this out. It will only prove helpful to others. 👍

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

2 participants