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

Add support for custom pagers, e.g. *Delta* #1593

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

Conversation

stevenxxiu
Copy link
Contributor

This PR resolves #97, to support custom pagers. I made it configurable and tested it with Delta.

For Delta, an example config is:

log_pager = { 'delta', '--width', '117' }

I used baleia to colorize text with ANSI escape sequences.

It's recommended to set disable_context_highlighting = true, otherwise when the cursor is in the hunk, we lose background highlighting.

To set a Delta theme, set the environment variables e.g.

vim.env.BAT_THEME = 'Catppuccin Latte'
vim.env.DELTA_FEATURES = '+catppuccin-latte'

@stevenxxiu stevenxxiu force-pushed the feat/custom-pager branch 2 times, most recently from 8dfdb1c to 7812a3f Compare December 8, 2024 15:40
@stevenxxiu stevenxxiu changed the title Adds support for custom pagers, e.g. *Delta* Add support for custom pagers, e.g. *Delta* Dec 8, 2024
lua/neogit/buffers/common.lua Outdated Show resolved Hide resolved
lua/neogit/lib/buffer.lua Outdated Show resolved Hide resolved
@CKolkey
Copy link
Member

CKolkey commented Dec 9, 2024

Very cool work - I didn't think it would be this straightforward.

@stevenxxiu stevenxxiu force-pushed the feat/custom-pager branch 2 times, most recently from bc6ab47 to 5bfc499 Compare December 12, 2024 08:29
@stevenxxiu stevenxxiu marked this pull request as draft December 12, 2024 08:34
@stevenxxiu stevenxxiu marked this pull request as ready for review December 12, 2024 09:46
@CKolkey
Copy link
Member

CKolkey commented Dec 13, 2024

Do you have a recommended set of flags to use with delta to provide a good output?
This is... a bit rough around the edges.

Screenshot 2024-12-13 at 16 29 57

@stevenxxiu
Copy link
Contributor Author

stevenxxiu commented Dec 13, 2024

Hm I'm not sure where the strange colors come from. These minimal settings should hopefully work:

I set these options:

  • disable_context_highlighting = true,
  • log_pager = { 'delta', '--width', '117' },

In Git settings:

[delta]
    side-by-side = true

See if that makes it display properly.

@stevenxxiu
Copy link
Contributor Author

stevenxxiu commented Dec 14, 2024

Ok I made a minimal config that's 100% reproducible with Docker. Maybe it's cause you didn't install Bat.

Create this Dockerfile:

# syntax = docker/dockerfile:1.2
FROM alpine:3.21.0

# Install packages
RUN apk add \
    bat \
    delta \
    git \
    neovim

Build and run it:

$ docker build --tag nvim_image .
$ docker container run --detach --rm --interactive --name nvim_container --mount 'type=bind,source=/tmp/nvim/,target=/tmp/nvim/' nvim_image
$ docker container exec --interactive --tty --workdir /tmp/nvim/ nvim_container /bin/sh -l

Create /tmp/nvim/init.lua with the contents:

-- Bootstrap lazy.nvim
local lazypath = vim.fn.stdpath('data') .. '/lazy/lazy.nvim'
if not (vim.uv or vim.loop).fs_stat(lazypath) then
  local lazyrepo = 'https://github.com/folke/lazy.nvim.git'
  local out = vim.fn.system({ 'git', 'clone', '--filter=blob:none', '--branch=stable', lazyrepo, lazypath })
  if vim.v.shell_error ~= 0 then
    vim.api.nvim_echo({
      { 'Failed to clone lazy.nvim:\n', 'ErrorMsg' },
      { out, 'WarningMsg' },
      { '\nPress any key to exit...' },
    }, true, {})
    vim.fn.getchar()
    os.exit(1)
  end
end
vim.opt.rtp:prepend(lazypath)

vim.g.mapleader = ' '
vim.g.maplocalleader = '\\'

vim.env.DELTA_FEATURES = '+side-by-side'

-- Setup lazy.nvim
require('lazy').setup({
  spec = {
    {
      dir = '/tmp/nvim/neogit/',
      dependencies = {
        'nvim-lua/plenary.nvim',
        'sindrets/diffview.nvim',
        'm00qek/baleia.nvim',
        'nvim-telescope/telescope.nvim',
      },
      opts = {
        disable_context_highlighting = true,
        graph_style = 'kitty',
        log_pager = { 'delta', '--width', '120' },
        commit_editor = {
          staged_diff_split_kind = 'vsplit_left',
        },
      },
    },
    {
      'm00qek/baleia.nvim',
      version = '*',
      config = function() vim.g.baleia = require('baleia').setup({}) end,
    },
  },
})

vim.cmd('Neogit cwd=/tmp/nvim/neogit/')

In the container run:

$ git clone -b feat/custom-pager --single-branch https://github.com/stevenxxiu/neogit.git
$ nvim -u init.lua

@stevenxxiu
Copy link
Contributor Author

Any luck on getting this to show properly on your end? I hope my Docker example helps. I can provide a screenshot if needed.

@CKolkey
Copy link
Member

CKolkey commented Dec 15, 2024

The docker example is 10/10, but it still looks off to me. Could you show me a screenshot of how it looks for you?

Screenshot 2024-12-15 at 22 12 19

@misaflo
Copy link

misaflo commented Dec 17, 2024

Hi,

I am trying your repository @stevenxxiu (thanks for the pull request!).

It works great with "side-by-side", but without it, I have ^[[0K at the end of lines.

neogit2

@stevenxxiu
Copy link
Contributor Author

stevenxxiu commented Dec 18, 2024

@misaflo I'm guessing baleia doesn't handle this ANSI control sequence. I removed that part of the line now.

@stevenxxiu
Copy link
Contributor Author

@CKolkey I finally figured the issue out. Turns out for baleia, I used version = "*" in init.lua, so the latest commit wasn't fetched. I'm guessing you did the same.

The following should work now.

Create this Dockerfile. I realize now less is enough. bat isn't required.

# syntax = docker/dockerfile:1.2
FROM alpine:3.21.0

# Install packages
RUN apk add \
    less \
    delta \
    git \
    neovim

Build and run it:

$ docker build --tag nvim_image .
$ docker container run --detach --rm --interactive --name nvim_container --mount 'type=bind,source=/tmp/nvim/,target=/tmp/nvim/' nvim_image
$ docker container exec --interactive --tty --workdir /tmp/nvim/ nvim_container /bin/sh -l

Create /tmp/nvim/init.lua with the contents:

-- Bootstrap lazy.nvim
local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not (vim.uv or vim.loop).fs_stat(lazypath) then
  local lazyrepo = "https://github.com/folke/lazy.nvim.git"
  local out = vim.fn.system({ "git", "clone", "--filter=blob:none", "--branch=stable", lazyrepo, lazypath })
  if vim.v.shell_error ~= 0 then
    vim.api.nvim_echo({
      { "Failed to clone lazy.nvim:\n", "ErrorMsg" },
      { out, "WarningMsg" },
      { "\nPress any key to exit..." },
    }, true, {})
    vim.fn.getchar()
    os.exit(1)
  end
end
vim.opt.rtp:prepend(lazypath)

vim.g.mapleader = " "
vim.g.maplocalleader = "\\"

vim.env.DELTA_FEATURES = "+side-by-side"

-- Setup lazy.nvim
require("lazy").setup({
  spec = {
    {
      dir = "/tmp/nvim/neogit/",
      dependencies = {
        "nvim-lua/plenary.nvim",
        "sindrets/diffview.nvim",
        "m00qek/baleia.nvim",
        "nvim-telescope/telescope.nvim",
      },
      opts = {
        disable_context_highlighting = true,
        graph_style = "kitty",
        log_pager = { "delta", "--width", "119" },
        commit_editor = {
          staged_diff_split_kind = "vsplit_left",
        },
      },
    },
    {
      "m00qek/baleia.nvim",
      config = function()
        vim.g.baleia = require("baleia").setup({})
      end,
    },
  },
})

vim.cmd("Neogit cwd=/tmp/nvim/neogit/")

In the container run:

$ git clone -b feat/custom-pager --single-branch https://github.com/stevenxxiu/neogit.git
$ nvim -u init.lua

@misaflo
Copy link

misaflo commented Dec 18, 2024

@misaflo I'm guessing baleia doesn't handle this ANSI control sequence. I removed that part of the line now.

It fixes the issue, but introduces an other: it highlights (greeen or red) an extra line, where there is no diff.

@stevenxxiu
Copy link
Contributor Author

stevenxxiu commented Dec 18, 2024

Ok I took a more detailed look. Now it shouldn't affect the next line, but it won't color the rest of the line's background either, like it should in the terminal. That requires baleia to properly support this ANSI escape sequence.

@stevenxxiu stevenxxiu marked this pull request as draft December 18, 2024 17:07
@stevenxxiu stevenxxiu marked this pull request as ready for review December 18, 2024 17:14
@stevenxxiu
Copy link
Contributor Author

Oh and here's some screenshots from the Docker config above:

Side by side:

Not side by side:

@stevenxxiu
Copy link
Contributor Author

I rebased and got unit tests to pass.

@CKolkey
Copy link
Member

CKolkey commented Dec 20, 2024

I'll try to do a good review soon as I can. I have some newborns (twin boys) at home right now so bear with me ;)

@stevenxxiu
Copy link
Contributor Author

Congrats on having 2 twin boys! Take your time.

@Daydreamer-riri
Copy link

This is a great work! Looking forward to the merger.

Add the `log_pager` option, which can be set to `delta`.
- This improves the speed.
- If not done, for large diffs we can get the error `PANIC: unprotected error in call to Lua API (EMFILE: too many open files)`.
…fer:set_ansi_highlights()`, to avoid finding hunk line numbers manually with the hard-coded `NeogitDiffContext`

- Add `ansi_hl` to `ComponentOptions`, to indicate whether the component needs ANSI highlighting.
- Add `ansi_highlight` to `RendererBuffer`, to pass hunk line numbers.
We don't support coloring the rest of the line.
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.

possible to integrate delta?
4 participants