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

Support delta along with ANSI support (continuation) #1298

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

pablospe
Copy link

The original pull request was closed due to the author deleting their account.

Original PR description

#542 (comment)

  • All test passes.
  • I haven't seen segmentation faults with newest versions of MacOS Big Sur, iTerm2, and delta. ncurses version is 6.2.
  • It looks like no performance issues.
  • I've only tested it with Terminal.app and iTerm on MacOS.

image

image

how to use delta in tig?

It requires ncurse 6.1 or higher.

# if you don't have ncurses
curl -O ftp://ftp.gnu.org/gnu/ncurses/ncurses-6.2.tar.gz
tar -xzvf ncurses-6.2.tar.gz
cd ncurses-6.2
./configure --prefix=${where you like} --enable-pc-files --with-pkg-config-libdir=/usr/local/bin/pkgconfig --enable-sigwinch --enable-symlinks --enable-widec --with-shared --with-gpm=no --without-ada --enable-ext-colors # i set the path as /usr/local which is default
sudo make
sudo make install

cd ${path to tig of the branch of above PR}
make configure
./configure LDFLAGS=-L${where you download ncurses}/lib CPPFLAGS=-I{where you installed ncurses}/include  # i set the path as /usr/local/opt/ncurses which is installed by ncurses
make

echo "set diff-highlight = \"delta\"" >> ~/.tigrc

./src/tig

Note: a more detailed installation instructions can be found in this comment from @adamency.

@rmacklin
Copy link

rmacklin commented Aug 5, 2024

Note for anyone landing on this PR that might want to try building and running it locally - this PR does not integrate delta's git blame support into tig blame. If you only want delta's git diff support in tig, this may work for you, but tig blame will still lack syntax highlighting.

@JelteF
Copy link
Contributor

JelteF commented Nov 4, 2024

Something is not really working correctly with this PR. It's still showing the regular diff view combined with the delta function header in many cases. This happens when I browse the branch of this PR with tig compiled from this branch.
image

To be clear, it doesn't always happen. Sometimes it will show the syntax highlighted diff of delta. But when scrolling through the commits with up and down arrows, most will show the old style diffs.

@vxsl
Copy link

vxsl commented Feb 9, 2025

Something is not really working correctly with this PR. It's still showing the regular diff view combined with the delta function header in many cases. This happens when I browse the branch of this PR with tig compiled from this branch.

To be clear, it doesn't always happen. Sometimes it will show the syntax highlighted diff of delta. But when scrolling through the commits with up and down arrows, most will show the old style diffs.

Just FYI I believe I have fixed this, will keep everybody posted over the next few days and hopefully submit a new PR.

@vxsl
Copy link

vxsl commented Feb 11, 2025

Ok @pablospe I created pablospe#1!

The issue seems to be that delta has buggy behaviour when --word-diff is used: dandavison/delta#1957. I noticed this because the output seems reliably correct in stage view, wherein --word-diff happens not to be used for whatever reason:

tig/include/tig/git.h

Lines 25 to 36 in b5f6193

#define GIT_DIFF_STAGED_INITIAL(encoding_arg, context_arg, space_arg, new_name) \
GIT_DIFF_INITIAL(encoding_arg, "--cached", context_arg, space_arg, "", new_name)
#define GIT_DIFF_STAGED(encoding_arg, context_arg, prefix_arg, space_arg, word_diff_arg, old_name, new_name) \
"git", "diff-index", (encoding_arg), "--textconv", "--patch-with-stat", "-C", \
"--cached", "--diff-filter=ACDMRTXB", DIFF_ARGS, "%(cmdlineargs)", (context_arg), \
(prefix_arg), (space_arg), (word_diff_arg), "HEAD", "--", (old_name), (new_name), NULL
#define GIT_DIFF_UNSTAGED(encoding_arg, context_arg, prefix_arg, space_arg, word_diff_arg, old_name, new_name) \
"git", "diff-files", (encoding_arg), "--textconv", "--patch-with-stat", "-C", \
DIFF_ARGS, "%(cmdlineargs)", (context_arg), (prefix_arg), (space_arg), (word_diff_arg), \
"--", (old_name), (new_name), NULL

The option is certainly not necessary when piping into delta, because it is not supported. So I just omitted it when delta is configured as the diff-higlighter.

Since this stems from some sort of race condition, can someone confirm that this change fixes the commit-diff-browsing issue (#1298 (comment)) on their machine as well? @JelteF? @pablospe?

@idvorkin
Copy link

Delta support would be great. Is there a plan to merge this PR back in?

@vxsl
Copy link

vxsl commented Feb 19, 2025

Delta support would be great. Is there a plan to merge this PR back in?

@idvorkin the bug currently being discussed above would be a blocker for merging it.

Also, there is another bug/missing feature that hasn't been discussed yet -- line/hunk staging is broken when using delta as the highlighter. So I would imagine this is also a blocker.

@vxsl
Copy link

vxsl commented Feb 23, 2025

Also, there is another bug/missing feature that hasn't been discussed yet -- line/hunk staging is broken when using delta as the highlighter. So I would imagine this is also a blocker.

Another one: pressing Enter on a file name in a diff results in "Failed to find file diff" instead of jumping to the relevant part of the diff as expected

@JelteF
Copy link
Contributor

JelteF commented Feb 23, 2025

Also, there is another bug/missing feature that hasn't been discussed yet -- line/hunk staging is broken when using delta as the highlighter.

Hmm, yeah that's indeed broken for me too. (I didn't use that before so I don't mind much honestly)

Another one: pressing Enter on a file name in a diff results in "Failed to find file diff" instead of jumping to the relevant part of the diff as expected

Also didn't use that often before, but that one can be worked I worked around by using --color-only with delta (that doesn't work for the staging thing though)

@JelteF
Copy link
Contributor

JelteF commented Feb 23, 2025

I'm notnow starting to use this PR as my daily driver now (including @vxsl fix to remove --word-diff). The main problem I have noticed currently is that it only supports ANSI colors, not full colors yet as tracked in #227, which makes insertions and deletions look the same in the catppuccin theme... But I've worked that by configuring a separate color for insertions and now. I'll let you know if I run into more issues.

@pablospe
Copy link
Author

Actually, this is what I am using myself at the moment (as work around) with delta, until this PR is ready. Two shortcuts: 'd' (in general) and 'D' (inside tmux)

.tigrc

#
# Bindings for Delta
#

# Open delta with git show.
bind generic d >sh -c "DELTA_PAGER='less -RKc' git show %(commit)"

# https://github.com/jonas/tig/issues/542#issuecomment-1242039892
bind generic D @sh -c "\
  ( \
    tmux has-session -t \".{last}\" \
    && tmux respawn-pane -t \".{last}\" -k 'LESS= DELTA_PAGER=\"less -R\" git show %(commit)' \
  ) \
  || tmux split-window -l 50% 'LESS= DELTA_PAGER=\"less -R\" git show %(commit)'"

.delta.gitconfig

[pager]
    diff = delta
    log = delta
    reflog = delta
    show = delta

[delta "colibri"]
    # author: https://github.com/pablospe
    # Based on woolly-mammoth: https://github.com/Kr1ss-XD.
    commit-decoration-style = 130 box
    dark = true
    file-decoration-style = box cyan bold ol ul 237
    file-added-label = [+]
    file-copied-label = [=]
    file-modified-label = [*]
    file-removed-label = [⛌]
    file-renamed-label = [→]
    file-style = cyan bold 237
    hunk-header-file-style = blue bold
    hunk-header-line-number-style = orange
    hunk-header-decoration-style = gray box ul
    hunk-header-style = file line-number syntax bold italic 237
    hunk-label = "@"
    line-numbers = true
    line-numbers-left-format = "{nm:>1}┊"
    line-numbers-left-style = red
    line-numbers-minus-style = red bold
    line-numbers-plus-style = green bold
    line-numbers-right-format = " {np:>1}┊"
    line-numbers-right-style = green
    line-numbers-zero-style = "#545474" italic
    minus-emph-style = syntax "#80002a"
    minus-style = syntax "#400000"
    plus-emph-style = syntax bold darkgreen
    plus-style = syntax "#003500"
    syntax-theme = OneHalfDark
    whitespace-error-style = black reverse
    zero-style = syntax
    blame-format = "{author:<18} ({commit:>7}) ┊{timestamp:^16}┊ "
    blame-palette = "#000000" "#003500" "#282828" "#3c3836"

[delta]
    features = colibri
    side-by-side = true
    navigate = true
    paging = auto
    # pager = less -RKc
    # https://github.com/dandavison/delta/blob/32a3994024b0a8f38c4e01fac2334945408e4e98/manual/src/tips-and-tricks/using-delta-with-vscode.md
    hyperlinks = true
    hyperlinks-file-link-format = "vscode://file/{path}:{line}"

[interactive]
    diffFilter = delta --color-only --features=interactive

.gitconfig

...
[include]
    path = .delta.gitconfig

@JelteF
Copy link
Contributor

JelteF commented Feb 28, 2025

I managed to get a consistent segfault using this branch when viewing this specific commit: ClickHouse/ClickBench@fdfdb5d

Backtrace:

(gdb) bt
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x000070e8b784527e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x000070e8b78288ff in __GI_abort () at ./stdlib/abort.c:79
#5  0x00005972eadb9ea8 in sigsegv_handler (sig=11) at src/tig.c:704
#6  <signal handler called>
#7  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#8  0x00005972eadef9e3 in draw_ansi (view=0x5972eae66000 <diff_view>, ansi_num=0x7ffc39842c28, ansi_ptrs=0x5973272cbcf0, max_width=116, skip=0) at src/ansi.c:72
#9  0x00005972eadccf98 in draw_chars_with_ansi (view=0x5972eae66000 <diff_view>, type=LINE_DEFAULT,
    string=0x5972eae916e0 <text> "\033[0m\033[48;5;52;38;5;247m],\033[38;5;189m \033[38;5;247m[\033[1;48;5;238;38;5;216m2\033[0m\033[48;5;52;38;5;247m.\033[1;48;5;238;38;5;216m177\033[0m\033[48;5;52;38;5;247m,\033[38;5;189m \033[38;5;216m1\033[38;5;247m.\033[1;48;5;238;38;5;2"..., length=-1, max_width=116, use_tilde=false) at src/draw.c:129
#10 0x00005972eadcd228 in draw_text_expanded (view=0x5972eae66000 <diff_view>, type=LINE_DEFAULT,
    string=0x5973272d008f "\033[0m\033[48;5;52;38;5;247m],\033[38;5;189m \033[38;5;247m[\033[1;48;5;238;38;5;216m2\033[0m\033[48;5;52;38;5;247m.\033[1;48;5;238;38;5;216m177\033[0m\033[48;5;52;38;5;247m,\033[38;5;189m \033[38;5;216m1\033[38;5;247m.\033[1;48;5;238;38;5;2"..., length=7087, max_width=116, use_tilde=false) at src/draw.c:180
#11 0x00005972eadcd33c in draw_textn (view=0x5972eae66000 <diff_view>, type=LINE_DEFAULT,
    string=0x5973272cfc90 "\033[48;5;52m-\033[38;5;247m[\033[38;5;216m0\033[38;5;247m.\033[1;48;5;238;38;5;216m055\033[0m\033[48;5;52;38;5;247m,\033[38;5;189m \033[38;5;216m0\033[38;5;247m.\033[1;48;5;238;38;5;216m017\033[0m\033[48;5;52;38;5;247m,\033[38;5;189m \033[38;5;"..., length=8110) at src/draw.c:202
#12 0x00005972eadcea89 in view_column_draw (view=0x5972eae66000 <diff_view>, line=0x5973272a7bc0, lineno=72) at src/draw.c:648
#13 0x00005972eadcef1c in draw_view_line (view=0x5972eae66000 <diff_view>, lineno=72) at src/draw.c:739
#14 0x00005972eadcf10b in redraw_view_from (view=0x5972eae66000 <diff_view>, lineno=72) at src/draw.c:783
#15 0x00005972eadd68b4 in update_view (view=0x5972eae66000 <diff_view>) at src/view.c:674
#16 0x00005972eadd4056 in update_views () at src/display.c:750
#17 0x00005972eadd41aa in get_input (prompt_position=-1, key=0x7ffc39842f64) at src/display.c:786
#18 0x00005972eadcf3ed in prompt_input (prompt=0x5972eadf8950 "", input=0x5972eae63d00 <incremental>) at src/prompt.c:55
#19 0x00005972eadcf9e8 in read_prompt_incremental (prompt=0x5972eadf8950 "", edit_mode=false, allow_empty=false, handler=0x5972eadb9ed6 <key_combo_handler>, data=0x7ffc398433e0) at src/prompt.c:202
#20 0x00005972eadba0e5 in read_key_combo (keymap=0x5972eae62180 <keymaps+96>) at src/tig.c:755
#21 0x00005972eadba64c in main (argc=2, argv=0x7ffc39843a68) at src/tig.c:866

@JelteF
Copy link
Contributor

JelteF commented Feb 28, 2025

Other issue I found. Very long lines overflow to the next lines somehow. Causing "white text" at the end of the line that they overflow to and when selecting these lines you will see more than one line being selected:
image

Example above is from this commit: duckdb/pg_duckdb@953ffc4

@JelteF
Copy link
Contributor

JelteF commented Feb 28, 2025

Issue number 3: Possibly related to the above. If you scroll to the right with the right arrow, then lines will lose their background color.

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

Successfully merging this pull request may close these issues.

5 participants