dandavison / delta

A syntax-highlighting pager for git, diff, grep, and blame output
https://dandavison.github.io/delta/
MIT License
24.38k stars 391 forks source link

🐛 Main thread panic when diffing file with spaces in its name #625

Closed ewen-lbh closed 3 years ago

ewen-lbh commented 3 years ago

I tried running git diff on "completely-rewritten file", because there are clearly a lot of lines that are common and a plain git diff (inside lazygit) wasn't cutting it (notably I was wondering if it wasn't a spaces v tab problem).

I'm running Manjaro 21.0.5 on kernel 5.12.2-1-MANJARO. I tried both the source and binary packages from the AUR (git-delta and git-delta-bin), as well as an install through cargo.

The raw git diff is quite large, so I put it at the end.

Here's the exception (from the cargo install, others had way less details in the stacktrace)

$ RUST_BACKTRACE=full git diff .config/Code\ -\ Insiders/User/settings.json
thread 'main' panicked at 'byte index 2 is out of bounds of `-`', /home/ewen/.cargo/registry/src/github.com-1ecc6299db9ec823/git-delta-0.8.0/src/parse.rs:82:47
stack backtrace:
   0:     0x558cfdadcf40 - std::backtrace_rs::backtrace::libunwind::trace::h5e9d00f0cdf4f57e
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/../../backtrace/src/backtrace/libunwind.rs:90:5
   1:     0x558cfdadcf40 - std::backtrace_rs::backtrace::trace_unsynchronized::hd5302bd66215dab9
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x558cfdadcf40 - std::sys_common::backtrace::_print_fmt::ha0237cd11a34e2bf
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:67:5
   3:     0x558cfdadcf40 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h171d4c10df1a98ee
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:46:22
   4:     0x558cfdafedec - core::fmt::write::h89e4288724daa3fa
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/fmt/mod.rs:1096:17
   5:     0x558cfdad9172 - std::io::Write::write_fmt::h6d40f996e84584d9
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/io/mod.rs:1568:15
   6:     0x558cfdadf005 - std::sys_common::backtrace::_print::h0c0b93221682afc8
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:49:5
   7:     0x558cfdadf005 - std::sys_common::backtrace::print::h57a9f95204c2fdd6
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:36:9
   8:     0x558cfdadf005 - std::panicking::default_hook::{{closure}}::h4245258b50e37e69
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:208:50
   9:     0x558cfdadeb63 - std::panicking::default_hook::h7b00dcc1d0944747
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:225:9
  10:     0x558cfdadf7a1 - std::panicking::rust_panic_with_hook::h71e6a073d87de1f5
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:591:17
  11:     0x558cfdadf2e7 - std::panicking::begin_panic_handler::{{closure}}::hd549436f6bb6dbb8
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:497:13
  12:     0x558cfdadd3dc - std::sys_common::backtrace::__rust_end_short_backtrace::h4e5f4b72b04174c3
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:141:18
  13:     0x558cfdadf249 - rust_begin_unwind
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
  14:     0x558cfdafd781 - core::panicking::panic_fmt::hcd56f7f635f62c74
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:92:14
  15:     0x558cfdb01fe5 - core::str::slice_error_fail::h3dfd55b8eb058100
  16:     0x558cfd90fd2f - delta::parse::get_file_paths_from_diff_line::h1a1f5dd249ffbdf2
  17:     0x558cfd9722de - delta::delta::StateMachine::handle_file_meta_diff_line::h6b2035457ff1e04a
  18:     0x558cfd970d71 - delta::delta::delta::hd4af7f02fd43ce4e
  19:     0x558cfd97cf01 - delta::run_app::h03624019d9a30a87
  20:     0x558cfd97f62c - delta::main::hed546d1efb17f02b
  21:     0x558cfd93d323 - std::sys_common::backtrace::__rust_begin_short_backtrace::h078c87e5cdde9722
  22:     0x558cfd93da89 - std::rt::lang_start::{{closure}}::hf49b4fa011f6eeea
  23:     0x558cfdadfcb7 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h527fb2333ede305e
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/ops/function.rs:259:13
  24:     0x558cfdadfcb7 - std::panicking::try::do_call::h309d8aee8149866c
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:379:40
  25:     0x558cfdadfcb7 - std::panicking::try::h75a60c31fd16bfc6
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:343:19
  26:     0x558cfdadfcb7 - std::panic::catch_unwind::h1f9892423e99bc00
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panic.rs:431:14
  27:     0x558cfdadfcb7 - std::rt::lang_start_internal::hd5b67df56ca01dae
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/rt.rs:51:25
  28:     0x558cfd983c22 - main
  29:     0x7f1ac6640b25 - __libc_start_main
  30:     0x558cfd8f81be - _start
  31:                0x0 - <unknown>

The raw diff:

diff --git a/.config/Code - Insiders/User/settings.json b/.config/Code - Insiders/User/settings.json
index 137d7eb..4519065 100644
--- a/.config/Code - Insiders/User/settings.json    
+++ b/.config/Code - Insiders/User/settings.json    
@@ -1,124 +1,132 @@
-{
-    "telemetry.enableTelemetry": false,
-    "editor.fontFamily": "Cartograph CF, Liga Dank Mono, Dank Mono, Twitter Color Emoji",
-    "workbench.iconTheme": "material-icon-theme",
-    "workbench.sideBar.location": "right",
-    "editor.minimap.enabled": false,
-    "files.autoSave": "afterDelay",
-    "terminal.integrated.shell.windows": "C:\\Windows\\System32\\wsl.exe",
-    "window.autoDetectColorScheme": true,
-    "workbench.preferredLightColorTheme": "Snazzy Light",
-    "workbench.preferredDarkColorTheme": "Palenight Italic",
-    "workbench.colorTheme": "Snazzy Light",
-    "i18n-ally.displayLanguage": "fr",
-    "editor.renderWhitespace": "none",
-    "svelte-autoimport.useSemiColon": false,
-    "editor.fontLigatures": true,
-    "git.autofetch": true,
-    "window.zoomLevel": 0,
-    "gitmoji.additionalEmojis": [
-        {
-            "code": ":mega:",
-            "description": "Logo, name or other brand resources changes",
-            "emoji": "📣"
-        },
-        {
-            "code": ":comet:",
-            "description": "Massive changes",
-            "emoji": "☄"
-        },
-        {
-            "code": ":hammer_and_wrench:",
-            "description": "Update developer's tooling",
-            "emoji": "🛠"
-        }
-    ],
-    "git.confirmSync": false,
-    "stylusSupremacy.insertSemicolons": false,
-    "stylusSupremacy.insertBraces": false,
-    "i18n-ally.indent": 4,
-    "i18n-ally.sortKeys": true,
-    "[markdown]": {
-        "editor.defaultFormatter": "yzhang.markdown-all-in-one"
-    },
-    "editor.renderFinalNewline": false,
-    "files.insertFinalNewline": true,
-    "files.trimFinalNewlines": true,
-    "githubIssues.workingIssueFormatScm": "${issueTitle} (Closes #${issueNumber})",
-    "explorer.openEditors.visible": 0,
-    "go.formatTool": "goimports",
-    "go.useLanguageServer": true,
-    "python.insidersChannel": "off",
-    "go.inferGopath": true,
-    "python.linting.pylintArgs": [
-        "--disable=unused-wildcard-import",
-        "--disable=wildcard-import",
-        "--disable=bad-continuation",
-        "--disable=unidiomatic-typecheck"
-    ],
-    "[go]": {
-        "editor.codeActionsOnSave": {
-            "source.fixAll": true
-        }
-    },
-    "[python]": {
-        "editor.codeActionsOnSave": {
-            "source.organizeImports": true
-        }
-    },
-    "editor.cursorSmoothCaretAnimation": true,
-    "githubIssues.issueCompletionFormatScm": "${issueTitle} (Closes #${issueNumber})",
-    "githubIssues.queries": [
-
-        {
-            "label": "My Issues",
-            "query": "default"
-        },
-        {
-            "label": "Created Issues",
-            "query": "author:${user} state:open repo:${owner}/${repository} sort:created-desc"
-        }
-    ],
-    "githubPullRequests.telemetry.enabled": false,
-    "diffEditor.ignoreTrimWhitespace": false,
-    "files.exclude": {
-        "**/__pycache__": true,
-        "**/*.egg-info": true
-    },
-    "todo-tree.tree.showScanModeButton": false,
-    "editor.codeLens": false,
-    "editor.fontSize": 16,
-    "sync.ignoredSettings": [
-        "editor.fontSize"
-    ],
-    "githubIssues.issueBranchTitle": "${sanitizedIssueTitle}",
-    "todo-tree.general.tags": [
-        "XXX",
-        "TODO",
-        "HACK",
-        "FIXME",
-        "BUG",
-        "obsolete"
-    ],
-    "python.formatting.provider": "black",
-    "terminal.integrated.fontSize": 16,
-    "emmet.excludeLanguages": [
-        "markdown",
-        "jade"
-    ],
-    "languageStylus.useSeparator": false,
-    "stylusSupremacy.insertColons": false,
-    "stylusSupremacy.insertParenthesisAroundIfCondition": false,
-    "stylusSupremacy.reduceMarginAndPaddingValues": true,
-    "stylusSupremacy.sortProperties": "grouped",
-    "markdown-toc.depthTo": 5,
-    "markdown.extension.toc.levels": "2..6",
-    "search.quickOpen.includeSymbols": true,
-    "githubIssues.useBranchForIssues": "prompt",
-    "files.associations": {
-        "*.txt": "plaintext"
-    },
-    "nim.logNimsuggest": true,
-    "vim.easymotion": true,
-    "editor.wordWrap": "on",
-}
+{
+  "telemetry.enableTelemetry": false,
+  "editor.fontFamily": "Cartograph CF, Liga Dank Mono, Dank Mono, Twitter Color Emoji",
+  "workbench.iconTheme": "material-icon-theme",
+  "workbench.sideBar.location": "right",
+  "editor.minimap.enabled": false,
+  "files.autoSave": "afterDelay",
+  "terminal.integrated.shell.windows": "C:\\Windows\\System32\\wsl.exe",
+  "window.autoDetectColorScheme": true,
+  "workbench.preferredLightColorTheme": "Snazzy Light",
+  "workbench.preferredDarkColorTheme": "Palenight Italic",
+  "workbench.colorTheme": "Snazzy Light",
+  "i18n-ally.displayLanguage": "fr",
+  "editor.renderWhitespace": "none",
+  "svelte-autoimport.useSemiColon": false,
+  "editor.fontLigatures": true,
+  "git.autofetch": true,
+  "window.zoomLevel": 0,
+  "gitmoji.additionalEmojis": [
+    {
+      "code": ":mega:",
+      "description": "Logo, name or other brand resources changes",
+      "emoji": "📣"
+    },
+    {
+      "code": ":comet:",
+      "description": "Massive changes",
+      "emoji": "☄"
+    },
+    {
+      "code": ":hammer_and_wrench:",
+      "description": "Update developer's tooling",
+      "emoji": "🛠"
+    }
+  ],
+  "git.confirmSync": false,
+  "stylusSupremacy.insertSemicolons": false,
+  "stylusSupremacy.insertBraces": false,
+  "i18n-ally.indent": 4,
+  "i18n-ally.sortKeys": true,
+  "[markdown]": {
+    "editor.defaultFormatter": "yzhang.markdown-all-in-one"
+  },
+  "editor.renderFinalNewline": false,
+  "files.insertFinalNewline": true,
+  "files.trimFinalNewlines": true,
+  "githubIssues.workingIssueFormatScm": "${issueTitle} (Closes #${issueNumber})",
+  "explorer.openEditors.visible": 0,
+  "go.formatTool": "goimports",
+  "go.useLanguageServer": true,
+  "python.insidersChannel": "off",
+  "go.inferGopath": true,
+  "python.linting.pylintArgs": [
+    "--disable=unused-wildcard-import",
+    "--disable=wildcard-import",
+    "--disable=bad-continuation",
+    "--disable=unidiomatic-typecheck"
+  ],
+  "[go]": {
+    "editor.codeActionsOnSave": {
+      "source.fixAll": true
+    }
+  },
+  "[python]": {
+    "editor.codeActionsOnSave": {
+      "source.organizeImports": true
+    }
+  },
+  "editor.cursorSmoothCaretAnimation": true,
+  "githubIssues.issueCompletionFormatScm": "${issueTitle} (Closes #${issueNumber})",
+  "githubIssues.queries": [
+    {
+      "label": "My Issues",
+      "query": "default"
+    },
+    {
+      "label": "Created Issues",
+      "query": "author:${user} state:open repo:${owner}/${repository} sort:created-desc"
+    }
+  ],
+  "githubPullRequests.telemetry.enabled": false,
+  "diffEditor.ignoreTrimWhitespace": false,
+  "files.exclude": {
+    "**/__pycache__": true,
+    "**/*.egg-info": true
+  },
+  "todo-tree.tree.showScanModeButton": false,
+  "editor.codeLens": false,
+  "editor.fontSize": 16,
+  "sync.ignoredSettings": [
+    "editor.fontSize"
+  ],
+  "githubIssues.issueBranchTitle": "${sanitizedIssueTitle}",
+  "todo-tree.general.tags": [
+    "XXX",
+    "TODO",
+    "HACK",
+    "FIXME",
+    "BUG",
+    "obsolete"
+  ],
+  "python.formatting.provider": "black",
+  "terminal.integrated.fontSize": 16,
+  "emmet.excludeLanguages": [
+    "markdown",
+    "jade"
+  ],
+  "languageStylus.useSeparator": false,
+  "stylusSupremacy.insertColons": false,
+  "stylusSupremacy.insertParenthesisAroundIfCondition": false,
+  "stylusSupremacy.reduceMarginAndPaddingValues": true,
+  "stylusSupremacy.sortProperties": "grouped",
+  "markdown-toc.depthTo": 5,
+  "markdown.extension.toc.levels": "2..6",
+  "search.quickOpen.includeSymbols": true,
+  "githubIssues.useBranchForIssues": "prompt",
+  "files.associations": {
+    "*.txt": "plaintext"
+  },
+  "nim.logNimsuggest": true,
+  "vim.easymotion": true,
+  "editor.wordWrap": "on",
+  "editor.suggestSelection": "first",
+  "vsintellicode.modify.editor.suggestSelection": "automaticallyOverrodeDefaultValue",
+  "workbench.editorAssociations": [
+    {
+      "viewType": "jupyter-notebook",
+      "filenamePattern": "*.ipynb"
+    }
+  ],
+  "go.toolsManagement.autoUpdate": true
+}

In case it is useful, my .gitconfig file:

[core]
    editor = nvim
    quotepath = off
    pager = delta
[user]
    email = hey@ewen.works
    name = Ewen Le Bihan
[init]
    defaultBranch = main
[credential "https://github.com"]
    helper = 
    helper = !/usr/bin/gh auth git-credential
[interactive]
    diffFilter = delta --color-only
[delta]
    line-numbers = true
ewen-lbh commented 3 years ago

On second thought, it might be because the filename has spaces (i.e. needs escaping) and that quotepath is turned off.

Still, quotepath = off is needed for me, else I can't use lazygit with non-ASCII filenames.

Changing the title.

ewen-lbh commented 3 years ago

Just tested it out: even with core.quotepath = on, delta still panics. Updating the title.

dandavison commented 3 years ago

Thanks very much @ewen-lbh for the detailed report. I can reproduce this (regression introduced at https://github.com/dandavison/delta/pull/605, failing to parse file names with spaces in diff --git ... line)

dandavison commented 3 years ago

A file named a/b is OK, so I've changed the title to "...when diffing file with spaces in its name".

dandavison commented 3 years ago

This has been fixed and released in 0.8.1

kidonng commented 3 years ago

The fix to this issue actually caused a regression for me:

~                                                                                                   
❯ delta --version
delta 0.8.1

~                                                                                                   
❯ delta ~/Library/Application\ Support/qBittorrent/Preferences/qBittorrent.ini ~/.config/qBittorrent/qBittorrent.ini
error: Found argument '/Users/kid/.config/qBittorrent/qBittorrent.ini' which wasn't expected, or isn't valid in this context

USAGE:
    delta [FLAGS] [OPTIONS] [ARGS]

For more information try --help

~                                                                                                   
❯ ./delta --version
delta 0.8.0

~                                                                                                   
❯ ./delta ~/Library/Application\ Support/qBittorrent/Preferences/qBittorrent.ini ~/.config/qBittorrent/qBittorrent.ini | head -n 5
diff --git a/Users/kid/Library/Application Support/qBittorrent/Preferences/qBittorrent.ini b/Users/kid/.config/qBittorrent/qBittorrent.ini
index afe22c1..a9d78ee 100644
--- a/Users/kid/Library/Application Support/qBittorrent/Preferences/qBittorrent.ini 
+++ b/Users/kid/.config/qBittorrent/qBittorrent.ini
@@ -1,3 +1,8 @@

Environment: fish 3.2.2 on macOS

dandavison commented 3 years ago

Hi @kidonng, thanks! That seems, at first glance, strange (but I can reproduce it). That's a command-line parsing error, and I don't immediately see what has changed between 0.8.0 and 0.8.1 that would cause a change in how positional filename arguments are parsed (versions of structopt and clap didn't change). Anyway, clearly I'm missing something. Someone shout out if they happen to see what that is!

dandavison commented 3 years ago

Ah, OK, it's this: https://github.com/dandavison/delta/commit/230f5468f7adddc098ab7b8cb241f06e497ae0d2 Some sort of shell escaping is needed if we're going to transmit arguments on to the child process like that.

dandavison commented 3 years ago

That's fixed in master.

kidonng commented 3 years ago

Awesome 👍

dandavison commented 3 years ago

Released in 0.8.2