From 65d176be35ccade0eef891e82e0af12929b0716f Mon Sep 17 00:00:00 2001 From: Pascal Kuthe <pascal.kuthe@semimod.de> Date: Sun, 6 Feb 2022 16:01:53 +0530 Subject: [PATCH 1/5] Show (git) diff signs in gutter (#3890) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid string allocation when git diffing Incrementally diff using changesets refactor diffs to be provider indepndent and improve git implementation remove dependency on zlib-ng switch to asynchronus diffing with similar Update helix-vcs/Cargo.toml fix toml formatting Co-authored-by: Ivan Tham <pickfire@riseup.net> fix typo in documentation use ropey reexpors from helix-core fix crash when creating new file remove useless use if io::Cursor fix spelling mistakes implement suggested improvement to repository loading improve git test isolation remove lefover comments Co-authored-by: univerz <univerz@fu-solution.com> fixed spelling mistake minor cosmetic changes fix: set self.differ to None if decoding the diff_base fails fixup formatting Co-authored-by: Ivan Tham <pickfire@riseup.net> reload diff_base when file is reloaded from disk switch to imara-diff Fixup formatting Co-authored-by: Blaž Hrastnik <blaz@mxxn.io> Redraw buffer whenever a diff is updated. Only store hunks instead of changes for individual lines to easily allow jumping between them Update to latest gitoxide version Change default diff gutter position Only update gutter after timeout --- Cargo.lock | 878 +++++++++++++++++++++++++++++- Cargo.toml | 1 + book/src/configuration.md | 2 +- helix-core/src/lib.rs | 2 +- helix-term/Cargo.toml | 3 + helix-term/src/application.rs | 6 +- helix-term/src/commands/typed.rs | 11 +- helix-term/src/ui/editor.rs | 2 +- helix-vcs/Cargo.toml | 25 + helix-vcs/src/diff.rs | 170 ++++++ helix-vcs/src/diff/line_cache.rs | 130 +++++ helix-vcs/src/diff/worker.rs | 95 ++++ helix-vcs/src/diff/worker/test.rs | 152 ++++++ helix-vcs/src/git.rs | 80 +++ helix-vcs/src/git/test.rs | 121 ++++ helix-vcs/src/lib.rs | 51 ++ helix-view/Cargo.toml | 2 + helix-view/src/document.rs | 51 +- helix-view/src/editor.rs | 55 +- helix-view/src/gutter.rs | 55 +- helix-view/src/view.rs | 15 +- 21 files changed, 1847 insertions(+), 60 deletions(-) create mode 100644 helix-vcs/Cargo.toml create mode 100644 helix-vcs/src/diff.rs create mode 100644 helix-vcs/src/diff/line_cache.rs create mode 100644 helix-vcs/src/diff/worker.rs create mode 100644 helix-vcs/src/diff/worker/test.rs create mode 100644 helix-vcs/src/git.rs create mode 100644 helix-vcs/src/git/test.rs create mode 100644 helix-vcs/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 05a0396fafdc..2e0211972bb6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,12 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "adler" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" + [[package]] name = "ahash" version = "0.7.6" @@ -55,6 +61,15 @@ version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "983cd8b9d4b02a6dc6ffa557262eb5858a27a0038ffffe21a0f133eaa819a164" +[[package]] +name = "atoi" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c57d12312ff59c811c0643f4d80830505833c9ffaebd193d819392b265be8e" +dependencies = [ + "num-traits", +] + [[package]] name = "autocfg" version = "1.1.0" @@ -78,12 +93,43 @@ dependencies = [ "regex-automata", ] +[[package]] +name = "bstr" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fca0852af221f458706eb0725c03e4ed6c46af9ac98e6a689d5e634215d594dd" +dependencies = [ + "memchr", + "once_cell", + "regex-automata", + "serde", +] + +[[package]] +name = "btoi" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97c0869a9faa81f8bbf8102371105d6d0a7b79167a04c340b04ab16892246a11" +dependencies = [ + "num-traits", +] + [[package]] name = "bumpalo" version = "3.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "572f695136211188308f16ad2ca5c851a712c464060ae6974944458eb83880ba" +[[package]] +name = "byte-unit" +version = "4.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "581ad4b3d627b0c09a0ccb2912148f839acaca0b93cf54cbe42b6c674e86079c" +dependencies = [ + "serde", + "utf8-width", +] + [[package]] name = "bytecount" version = "0.6.3" @@ -96,12 +142,27 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dfb24e866b15a1af2a1b663f10c6b6b8f397a84aadb828f12e5b289ec23a3a3c" +[[package]] +name = "bytesize" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c58ec36aac5066d5ca17df51b3e70279f5670a72102f5752cb7e7c856adfc70" + [[package]] name = "cassowary" version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df8670b8c7b9dae1793364eafadf7239c40d669904660c5960d74cfd80b46a53" +[[package]] +name = "castaway" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a17ed5635fc8536268e5d4de1e22e81ac34419e5f052d4d51f4e01dcc263fcc" +dependencies = [ + "rustversion", +] + [[package]] name = "cc" version = "1.0.77" @@ -148,6 +209,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "clru" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "218d6bd3dde8e442a975fa1cd233c0e5fded7596bccfe39f58eca98d22421e0a" + [[package]] name = "codespan-reporting" version = "0.11.1" @@ -158,6 +225,17 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "compact_str" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5138945395949e7dfba09646dc9e766b548ff48e23deb5246890e6b64ae9e1b9" +dependencies = [ + "castaway", + "itoa", + "ryu", +] + [[package]] name = "content_inspector" version = "0.2.4" @@ -173,6 +251,15 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5827cebf4670468b8772dd191856768aedcb1b0278a04f989f7766351917b9dc" +[[package]] +name = "crc32fast" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b540bd8bc810d3885c6ea91e2018302f68baba2129ab3e88f32389ee9370880d" +dependencies = [ + "cfg-if", +] + [[package]] name = "crossbeam-utils" version = "0.8.14" @@ -252,6 +339,28 @@ dependencies = [ "syn", ] +[[package]] +name = "dashmap" +version = "5.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "907076dfda823b0b36d2a1bb5f90c96660a5bbcd7729e10727f07858f22c4edc" +dependencies = [ + "cfg-if", + "hashbrown 0.12.3", + "lock_api", + "once_cell", + "parking_lot_core", +] + +[[package]] +name = "dirs" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca3aa72a6f96ea37bbc5aa912f6788242832f75369bdfdadcb0e38423f100059" +dependencies = [ + "dirs-sys", +] + [[package]] name = "dirs-next" version = "2.0.0" @@ -262,6 +371,17 @@ dependencies = [ "dirs-sys-next", ] +[[package]] +name = "dirs-sys" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b1d1d91c932ef41c0f2663aa8b0ca0342d444d842c06914aa0a7e352d0bada6" +dependencies = [ + "libc", + "redox_users", + "winapi", +] + [[package]] name = "dirs-sys-next" version = "0.1.2" @@ -336,6 +456,28 @@ dependencies = [ "log", ] +[[package]] +name = "filetime" +version = "0.2.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b9663d381d07ae25dc88dbdf27df458faa83a9b25336bcac83d5e452b5fc9d3" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "windows-sys", +] + +[[package]] +name = "flate2" +version = "1.0.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8a2db397cb1c8772f31494cb8917e48cd1e64f0fa7efac59fbd741a0a8ce841" +dependencies = [ + "crc32fast", + "miniz_oxide", +] + [[package]] name = "fnv" version = "1.0.7" @@ -407,6 +549,498 @@ dependencies = [ "wasi", ] +[[package]] +name = "git-actor" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18d4ce09c0a6c71c044700e5932877667f427f007b77e6c39ab49aebc4719e25" +dependencies = [ + "bstr 1.0.1", + "btoi", + "git-date", + "itoa", + "nom", + "quick-error", +] + +[[package]] +name = "git-attributes" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c62e66a042c6b39c6dbfa3be37d134900d99ff9c54bbe489ed560a573895d5d" +dependencies = [ + "bstr 1.0.1", + "compact_str", + "git-features", + "git-glob", + "git-path", + "git-quote", + "thiserror", + "unicode-bom", +] + +[[package]] +name = "git-bitmap" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "327098a7ad27ae298d7e71602dbd4375cc828d755d10a720e4be0be1b4ec38f0" +dependencies = [ + "quick-error", +] + +[[package]] +name = "git-chunk" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07b2bc1635b660ad6e30379a84a4946590a3c124b747107c2cca1d9dbb98f588" +dependencies = [ + "thiserror", +] + +[[package]] +name = "git-command" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e4b01997b6551554fdac6f02277d0d04c3e869daa649bedd06d38c86f11dc42" +dependencies = [ + "bstr 1.0.1", +] + +[[package]] +name = "git-config" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd8603e953bd4c9bf310e74e43697400f5542f1cc75fad46fbd7427135a9534f" +dependencies = [ + "bstr 1.0.1", + "git-config-value", + "git-features", + "git-glob", + "git-path", + "git-ref", + "git-sec", + "memchr", + "nom", + "once_cell", + "smallvec", + "thiserror", + "unicode-bom", +] + +[[package]] +name = "git-config-value" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f276bfe5806b414915112f1eec0f006206cdf5b8cc9bbb44ef7e52286dc3eb" +dependencies = [ + "bitflags", + "bstr 1.0.1", + "git-path", + "libc", + "thiserror", +] + +[[package]] +name = "git-credentials" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f540186ea56fd075ba2b923180ebf4318e66ceaeac0a2a518e75dab8517d339" +dependencies = [ + "bstr 1.0.1", + "git-command", + "git-config-value", + "git-path", + "git-prompt", + "git-sec", + "git-url", + "thiserror", +] + +[[package]] +name = "git-date" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "37881e9725df41e15d16216d3a0cee251fd8a39d425f75b389112df5c7f20f3d" +dependencies = [ + "bstr 1.0.1", + "itoa", + "thiserror", + "time", +] + +[[package]] +name = "git-diff" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a88666a0ae4365b55a0cbf2efde68d2a4cff0747894ad229403bd60b0b2abc5" +dependencies = [ + "git-hash", + "git-object", + "imara-diff", + "thiserror", +] + +[[package]] +name = "git-discover" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "881e4136d5599cfdb79d8ef60d650823d1a563589fa493d8e4961e64d78a79f2" +dependencies = [ + "bstr 1.0.1", + "git-hash", + "git-path", + "git-ref", + "git-sec", + "thiserror", +] + +[[package]] +name = "git-features" +version = "0.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4be88ae837674c71b30c6517c6f5f1335f8135bb8a9ffef20000d211933bed08" +dependencies = [ + "crc32fast", + "flate2", + "git-hash", + "libc", + "once_cell", + "prodash", + "quick-error", + "sha1_smol", + "walkdir", +] + +[[package]] +name = "git-glob" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d756430237112f8c89049236f60fdcdb0005127b1f7e531d40984e4fe7daa90" +dependencies = [ + "bitflags", + "bstr 1.0.1", +] + +[[package]] +name = "git-hash" +version = "0.9.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16d46e6c2d1e8da4438a87bf516a6761b300964a353541fea61e96b3c7b34554" +dependencies = [ + "hex", + "thiserror", +] + +[[package]] +name = "git-index" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "821583c2d12b1e864694eb0bf1cca10ff6a3f45966f5f834e0f921b496dbe7cb" +dependencies = [ + "atoi", + "bitflags", + "bstr 1.0.1", + "filetime", + "git-bitmap", + "git-features", + "git-hash", + "git-lock", + "git-object", + "git-traverse", + "itoa", + "memmap2", + "smallvec", + "thiserror", +] + +[[package]] +name = "git-lock" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f0fe10bf961f62b1335b4c07785e64fb4d86c5ed367dc7cd9360f13c3eb7c78" +dependencies = [ + "fastrand", + "git-tempfile", + "quick-error", +] + +[[package]] +name = "git-mailmap" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb3f85ce84b2328aeb3124a809f7b3a63e59c4d63c227dba7a9cdf6fca6c0987" +dependencies = [ + "bstr 1.0.1", + "git-actor", + "quick-error", +] + +[[package]] +name = "git-object" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9469a8c00d8bb500ee76a12e455bb174b4ddf71674713335dd1a84313723f7b3" +dependencies = [ + "bstr 1.0.1", + "btoi", + "git-actor", + "git-features", + "git-hash", + "git-validate", + "hex", + "itoa", + "nom", + "smallvec", + "thiserror", +] + +[[package]] +name = "git-odb" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aaaea7031ac7d8dfee232a16d7114395d118226214fb03fe4e15d1f4d62a88a6" +dependencies = [ + "arc-swap", + "git-features", + "git-hash", + "git-object", + "git-pack", + "git-path", + "git-quote", + "parking_lot", + "tempfile", + "thiserror", +] + +[[package]] +name = "git-pack" +version = "0.25.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc4386dff835ffdc3697c3558111f708fd7b7695c42a4347f2d211cf3246c8e1" +dependencies = [ + "bytesize", + "clru", + "dashmap", + "git-chunk", + "git-diff", + "git-features", + "git-hash", + "git-object", + "git-path", + "git-tempfile", + "git-traverse", + "hash_hasher", + "memmap2", + "parking_lot", + "smallvec", + "thiserror", +] + +[[package]] +name = "git-path" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "425dc1022690be13e6c5bde4b7e04d9504d323605ec314cd367cebf38a812572" +dependencies = [ + "bstr 1.0.1", + "thiserror", +] + +[[package]] +name = "git-prompt" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa6947935c0671342277bc883ff0687978477b570c1ffe2200b9ba5ac8afdd9f" +dependencies = [ + "git-command", + "git-config-value", + "nix", + "parking_lot", + "thiserror", +] + +[[package]] +name = "git-quote" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ea17931d07cbe447f371bbdf45ff03c30ea86db43788166655a5302df87ecfc" +dependencies = [ + "bstr 1.0.1", + "btoi", + "quick-error", +] + +[[package]] +name = "git-ref" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "638c9e454bacb2965a43f05b4a383c8f66dc64f3a770bd0324b221c2a20e121d" +dependencies = [ + "git-actor", + "git-features", + "git-hash", + "git-lock", + "git-object", + "git-path", + "git-tempfile", + "git-validate", + "memmap2", + "nom", + "thiserror", +] + +[[package]] +name = "git-refspec" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9497af773538ae8cfda053ff7dd0a9e6c28d333ba653040f54b8b4ee32f14187" +dependencies = [ + "bstr 1.0.1", + "git-hash", + "git-revision", + "git-validate", + "smallvec", + "thiserror", +] + +[[package]] +name = "git-repository" +version = "0.26.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eeb43e59612e493af6a433bf0a960de0042c8aa6f4e4c4cb414f03b97e296b82" +dependencies = [ + "byte-unit", + "clru", + "git-actor", + "git-attributes", + "git-config", + "git-credentials", + "git-date", + "git-diff", + "git-discover", + "git-features", + "git-glob", + "git-hash", + "git-index", + "git-lock", + "git-mailmap", + "git-object", + "git-odb", + "git-pack", + "git-path", + "git-prompt", + "git-ref", + "git-refspec", + "git-revision", + "git-sec", + "git-tempfile", + "git-traverse", + "git-url", + "git-validate", + "git-worktree", + "log", + "once_cell", + "signal-hook", + "smallvec", + "thiserror", + "unicode-normalization", +] + +[[package]] +name = "git-revision" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1efd31c63c3745b5dba5ec7109eec41a9c717f4e1e797fe0ef93098f33f31b25" +dependencies = [ + "bstr 1.0.1", + "git-date", + "git-hash", + "git-object", + "hash_hasher", + "thiserror", +] + +[[package]] +name = "git-sec" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8c79769f6546814d0774db7295c768441016b7e40bdd414fa8dfae2c616a1892" +dependencies = [ + "bitflags", + "dirs", + "git-path", + "libc", + "windows", +] + +[[package]] +name = "git-tempfile" +version = "2.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d23bc6129de3cbd81e6c9d0d685b5540c6b41bd9fa0cc38f381bc300743d708" +dependencies = [ + "dashmap", + "libc", + "once_cell", + "signal-hook", + "signal-hook-registry", + "tempfile", +] + +[[package]] +name = "git-traverse" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d0c4dd773c69f294f43ace8373d48eb770129791f104c6857fa8cac0505af89" +dependencies = [ + "git-hash", + "git-object", + "hash_hasher", + "thiserror", +] + +[[package]] +name = "git-url" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21b7f8323196840e7932f5b60e1d9c1d6c140fd806bc512f8beedc3f990a1f81" +dependencies = [ + "bstr 1.0.1", + "git-features", + "git-path", + "home", + "thiserror", + "url", +] + +[[package]] +name = "git-validate" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5439d6aa0de838dfadd74a71e97a9e23ebc719fd11a9ab6788b835b112c8c3d" +dependencies = [ + "bstr 1.0.1", + "thiserror", +] + +[[package]] +name = "git-worktree" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "45bcc69c36a29cfa283710b7901877ab251d658935f5a41ed824416af500e0ed" +dependencies = [ + "bstr 1.0.1", + "git-attributes", + "git-features", + "git-glob", + "git-hash", + "git-index", + "git-object", + "git-path", + "io-close", + "thiserror", +] + [[package]] name = "globset" version = "0.4.9" @@ -414,7 +1048,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0a1e17342619edbc21a964c2afbeb6c820c6a2560032872f397bb97ea127bd0a" dependencies = [ "aho-corasick", - "bstr", + "bstr 0.2.17", "fnv", "log", "regex", @@ -436,7 +1070,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1345f8d33c89f2d5b081f2f2a41175adef9fd0bed2fea6a26c96c2deb027e58e" dependencies = [ "aho-corasick", - "bstr", + "bstr 0.2.17", "grep-matcher", "log", "regex", @@ -450,7 +1084,7 @@ version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "48852bd08f9b4eb3040ecb6d2f4ade224afe880a9a0909c5563cc59fa67932cc" dependencies = [ - "bstr", + "bstr 0.2.17", "bytecount", "encoding_rs", "encoding_rs_io", @@ -459,6 +1093,12 @@ dependencies = [ "memmap2", ] +[[package]] +name = "hash_hasher" +version = "2.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74721d007512d0cb3338cd20f0654ac913920061a4c4d0d8708edb3f2a698c0c" + [[package]] name = "hashbrown" version = "0.12.3" @@ -576,6 +1216,7 @@ dependencies = [ "helix-loader", "helix-lsp", "helix-tui", + "helix-vcs", "helix-view", "ignore", "indoc", @@ -608,6 +1249,19 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "helix-vcs" +version = "0.6.0" +dependencies = [ + "git-repository", + "helix-core", + "imara-diff", + "log", + "parking_lot", + "tempfile", + "tokio", +] + [[package]] name = "helix-view" version = "0.6.0" @@ -624,6 +1278,7 @@ dependencies = [ "helix-loader", "helix-lsp", "helix-tui", + "helix-vcs", "log", "once_cell", "serde", @@ -645,6 +1300,27 @@ dependencies = [ "libc", ] +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + +[[package]] +name = "home" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "747309b4b440c06d57b0b25f2aee03ee9b5e5397d288c60e21fc709bb98a7408" +dependencies = [ + "winapi", +] + +[[package]] +name = "human_format" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86cce260d758a9aa3d7c4b99d55c815a540f8a37514ba6046ab6be402a157cb0" + [[package]] name = "iana-time-zone" version = "0.1.53" @@ -722,6 +1398,16 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "io-close" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9cadcf447f06744f8ce713d2d6239bb5bde2c357a452397a9ed90c625da390bc" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "itoa" version = "1.0.4" @@ -815,6 +1501,21 @@ dependencies = [ "libc", ] +[[package]] +name = "minimal-lexical" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" + +[[package]] +name = "miniz_oxide" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b275950c28b37e794e8c55d88aeb5e139d0ce23fdbbeda68f8d7174abdf9e8fa" +dependencies = [ + "adler", +] + [[package]] name = "mio" version = "0.8.5" @@ -827,6 +1528,28 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "nix" +version = "0.25.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e322c04a9e3440c327fca7b6c8a63e6890a32fa2ad689db972425f07e0d22abb" +dependencies = [ + "autocfg", + "bitflags", + "cfg-if", + "libc", +] + +[[package]] +name = "nom" +version = "7.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8903e5a29a317527874d0402f867152a3d21c908bb0b933e416c65e301d4c36" +dependencies = [ + "memchr", + "minimal-lexical", +] + [[package]] name = "num-integer" version = "0.1.45" @@ -856,6 +1579,15 @@ dependencies = [ "libc", ] +[[package]] +name = "num_threads" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2819ce041d2ee131036f4fc9d6ae7ae125a3a40e97ba64d04fe799ad9dabbb44" +dependencies = [ + "libc", +] + [[package]] name = "once_cell" version = "1.16.0" @@ -912,6 +1644,16 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "prodash" +version = "21.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e13d7bd38cdab08b3a8b780cedcc54238c84fdca4084eb188807b308bcf11e6" +dependencies = [ + "bytesize", + "human_format", +] + [[package]] name = "pulldown-cmark" version = "0.9.2" @@ -923,6 +1665,12 @@ dependencies = [ "unicase", ] +[[package]] +name = "quick-error" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" + [[package]] name = "quickcheck" version = "1.0.3" @@ -1021,6 +1769,12 @@ dependencies = [ "str_indices", ] +[[package]] +name = "rustversion" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97477e48b4cf8603ad5f7aaf897467cf42ab4218a38ef76fb14c2d6773a6d6a8" + [[package]] name = "ryu" version = "1.0.11" @@ -1090,6 +1844,12 @@ dependencies = [ "syn", ] +[[package]] +name = "sha1_smol" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae1a47186c03a32177042e55dbc5fd5aee900b8e0069a8d70fba96a9375cd012" + [[package]] name = "signal-hook" version = "0.3.14" @@ -1293,6 +2053,35 @@ dependencies = [ "num_cpus", ] +[[package]] +name = "time" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a561bf4617eebd33bca6434b988f39ed798e527f51a1e797d0ee4f61c0a38376" +dependencies = [ + "itoa", + "libc", + "num_threads", + "serde", + "time-core", + "time-macros", +] + +[[package]] +name = "time-core" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e153e1f1acaef8acc537e68b44906d2db6436e2b35ac2c6b42640fff91f00fd" + +[[package]] +name = "time-macros" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d967f99f534ca7e495c575c62638eebc2898a8c84c119b89e250477bc4ba16b2" +dependencies = [ + "time-core", +] + [[package]] name = "tinyvec" version = "1.6.0" @@ -1384,6 +2173,12 @@ version = "0.3.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "099b7128301d285f79ddd55b9a83d5e6b9e97c92e0ea0daebee7263e932de992" +[[package]] +name = "unicode-bom" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63ec69f541d875b783ca40184d655f2927c95f0bffd486faa83cd3ac3529ec32" + [[package]] name = "unicode-general-category" version = "0.6.0" @@ -1439,6 +2234,12 @@ dependencies = [ "serde", ] +[[package]] +name = "utf8-width" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5190c9442dcdaf0ddd50f37420417d219ae5261bbf5db120d0f9bab996c9cba1" + [[package]] name = "version_check" version = "0.9.4" @@ -1558,57 +2359,114 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e30acc718a52fb130fec72b1cb5f55ffeeec9253e1b785e94db222178a6acaa1" +dependencies = [ + "windows_aarch64_gnullvm 0.40.0", + "windows_aarch64_msvc 0.40.0", + "windows_i686_gnu 0.40.0", + "windows_i686_msvc 0.40.0", + "windows_x86_64_gnu 0.40.0", + "windows_x86_64_gnullvm 0.40.0", + "windows_x86_64_msvc 0.40.0", +] + [[package]] name = "windows-sys" version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5a3e1820f08b8513f676f7ab6c1f99ff312fb97b553d30ff4dd86f9f15728aa7" dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", + "windows_aarch64_gnullvm 0.42.0", + "windows_aarch64_msvc 0.42.0", + "windows_i686_gnu 0.42.0", + "windows_i686_msvc 0.42.0", + "windows_x86_64_gnu 0.42.0", + "windows_x86_64_gnullvm 0.42.0", + "windows_x86_64_msvc 0.42.0", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3caa4a1a16561b714323ca6b0817403738583033a6a92e04c5d10d4ba37ca10" + [[package]] name = "windows_aarch64_gnullvm" version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "41d2aa71f6f0cbe00ae5167d90ef3cfe66527d6f613ca78ac8024c3ccab9a19e" +[[package]] +name = "windows_aarch64_msvc" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "328973c62dfcc50fb1aaa8e7100676e0b642fe56bac6bafff3327902db843ab4" + [[package]] name = "windows_aarch64_msvc" version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd0f252f5a35cac83d6311b2e795981f5ee6e67eb1f9a7f64eb4500fbc4dcdb4" +[[package]] +name = "windows_i686_gnu" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa5b09fad70f0df85dea2ac2a525537e415e2bf63ee31cf9b8e263645ee9f3c1" + [[package]] name = "windows_i686_gnu" version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fbeae19f6716841636c28d695375df17562ca208b2b7d0dc47635a50ae6c5de7" +[[package]] +name = "windows_i686_msvc" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a1ad4031c1a98491fa195d8d43d7489cb749f135f2e5c4eed58da094bd0d876" + [[package]] name = "windows_i686_msvc" version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "84c12f65daa39dd2babe6e442988fc329d6243fdce47d7d2d155b8d874862246" +[[package]] +name = "windows_x86_64_gnu" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "520ff37edd72da8064b49d2281182898e17f0688ae9f4070bca27e4b5c162ac7" + [[package]] name = "windows_x86_64_gnu" version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf7b1b21b5362cbc318f686150e5bcea75ecedc74dd157d874d754a2ca44b0ed" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "046e5b82215102c44fd75f488f1b9158973d02aa34d06ed85c23d6f5520a2853" + [[package]] name = "windows_x86_64_gnullvm" version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09d525d2ba30eeb3297665bd434a54297e4170c7f1a44cad4ef58095b4cd2028" +[[package]] +name = "windows_x86_64_msvc" +version = "0.40.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a0c9c6df55dd1bfa76e131cef44bdd8ec9c819ef3611f04dfe453fd5bfeda28" + [[package]] name = "windows_x86_64_msvc" version = "0.42.0" diff --git a/Cargo.toml b/Cargo.toml index 9e985ddcd1c4..ecf6848e04a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ members = [ "helix-lsp", "helix-dap", "helix-loader", + "helix-vcs", "xtask", ] diff --git a/book/src/configuration.md b/book/src/configuration.md index e4854cda1a70..0890d28328ca 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -46,7 +46,7 @@ on unix operating systems. | `line-number` | Line number display: `absolute` simply shows each line's number, while `relative` shows the distance from the current line. When unfocused or in insert mode, `relative` will still show absolute line numbers. | `absolute` | | `cursorline` | Highlight all lines with a cursor. | `false` | | `cursorcolumn` | Highlight all columns with a cursor. | `false` | -| `gutters` | Gutters to display: Available are `diagnostics` and `line-numbers` and `spacer`, note that `diagnostics` also includes other features like breakpoints, 1-width padding will be inserted if gutters is non-empty | `["diagnostics", "spacer", "line-numbers"]` | +| `gutters` | Gutters to display: Available are `diagnostics` and `diff` and `line-numbers` and `spacer`, note that `diagnostics` also includes other features like breakpoints, 1-width padding will be inserted if gutters is non-empty | `["diagnostics", "spacer", "line-numbers", "spacer", "diff"]` | | `auto-completion` | Enable automatic pop up of auto-completion. | `true` | | `auto-format` | Enable automatic formatting on save. | `true` | | `auto-save` | Enable automatic saving on focus moving away from Helix. Requires [focus event support](https://github.com/helix-editor/helix/wiki/Terminal-Support) from your terminal. | `false` | diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index 5f60c04890de..0e76ebbbefca 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -83,7 +83,7 @@ pub fn find_root(root: Option<&str>, root_markers: &[String]) -> std::path::Path top_marker.map_or(current_dir, |a| a.to_path_buf()) } -pub use ropey::{str_utils, Rope, RopeBuilder, RopeSlice}; +pub use ropey::{self, str_utils, Rope, RopeBuilder, RopeSlice}; // pub use tendril::StrTendril as Tendril; pub use smartstring::SmartString; diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 485cabe90819..30bfc7ea38a3 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -17,8 +17,10 @@ build = true app = true [features] +default = ["git"] unicode-lines = ["helix-core/unicode-lines"] integration = [] +git = ["helix-vcs/git"] [[bin]] name = "hx" @@ -29,6 +31,7 @@ helix-core = { version = "0.6", path = "../helix-core" } helix-view = { version = "0.6", path = "../helix-view" } helix-lsp = { version = "0.6", path = "../helix-lsp" } helix-dap = { version = "0.6", path = "../helix-dap" } +helix-vcs = { version = "0.6", path = "../helix-vcs" } helix-loader = { version = "0.6", path = "../helix-loader" } anyhow = "1" diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 99d3af182edf..ec377689e487 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -31,7 +31,7 @@ use crate::{ use log::{debug, error, warn}; use std::{ io::{stdin, stdout, Write}, - sync::Arc, + sync::{atomic::Ordering, Arc}, time::{Duration, Instant}, }; @@ -284,6 +284,8 @@ impl Application { scroll: None, }; + cx.editor.redraw_handle.store(false, Ordering::Relaxed); + let area = self .terminal .autoresize() @@ -462,7 +464,7 @@ impl Application { scroll: None, }; let should_render = self.compositor.handle_event(&Event::IdleTimeout, &mut cx); - if should_render { + if should_render || self.editor.redraw_handle.load(Ordering::Relaxed) { self.render(); } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 351692fd24df..ecd34885cb41 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1028,10 +1028,12 @@ fn reload( } let scrolloff = cx.editor.config().scrolloff; + let redraw_handle = cx.editor.redraw_handle.clone(); let (view, doc) = current!(cx.editor); - doc.reload(view).map(|_| { - view.ensure_cursor_in_view(doc, scrolloff); - }) + doc.reload(view, &cx.editor.diff_providers, redraw_handle) + .map(|_| { + view.ensure_cursor_in_view(doc, scrolloff); + }) } fn reload_all( @@ -1066,7 +1068,8 @@ fn reload_all( // Every doc is guaranteed to have at least 1 view at this point. let view = view_mut!(cx.editor, view_ids[0]); - doc.reload(view)?; + let redraw_handle = cx.editor.redraw_handle.clone(); + doc.reload(view, &cx.editor.diff_providers, redraw_handle)?; for view_id in view_ids { let view = view_mut!(cx.editor, view_id); diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 737125031e99..a6b7f7136fed 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -730,7 +730,7 @@ impl EditorView { let mut text = String::with_capacity(8); for gutter_type in view.gutters() { - let gutter = gutter_type.style(editor, doc, view, theme, is_focused); + let mut gutter = gutter_type.style(editor, doc, view, theme, is_focused); let width = gutter_type.width(view, doc); text.reserve(width); // ensure there's enough space for the gutter for (i, line) in (view.offset.row..(last_line + 1)).enumerate() { diff --git a/helix-vcs/Cargo.toml b/helix-vcs/Cargo.toml new file mode 100644 index 000000000000..f2b1a3f7e416 --- /dev/null +++ b/helix-vcs/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "helix-vcs" +version = "0.6.0" +authors = ["Blaž Hrastnik <blaz@mxxn.io>"] +edition = "2021" +license = "MPL-2.0" +categories = ["editor"] +repository = "https://github.com/helix-editor/helix" +homepage = "https://helix-editor.com" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +git-repository = { version = "0.26", default-features = false , optional = true } +tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "parking_lot", "macros"] } +imara-diff = "0.1.5" +helix-core = { version = "0.6", path = "../helix-core" } +log = "0.4" +parking_lot = "0.12" + +[features] +git = ["git-repository"] + +[dev-dependencies] +tempfile = "3.3" \ No newline at end of file diff --git a/helix-vcs/src/diff.rs b/helix-vcs/src/diff.rs new file mode 100644 index 000000000000..084c725f58e4 --- /dev/null +++ b/helix-vcs/src/diff.rs @@ -0,0 +1,170 @@ +use std::ops::Range; +use std::sync::atomic::AtomicBool; +use std::sync::Arc; + +use helix_core::Rope; +use imara_diff::Algorithm; +use parking_lot::{Mutex, MutexGuard}; +use tokio::sync::mpsc::{unbounded_channel, UnboundedSender}; +use tokio::task::JoinHandle; + +use crate::diff::worker::DiffWorker; + +mod line_cache; +mod worker; + +enum Event { + UpdateDocument(Rope), + UpdateDiffBase(Rope), +} + +#[derive(Clone, Debug)] +pub struct DiffHandle { + channel: UnboundedSender<Event>, + hunks: Arc<Mutex<Vec<Hunk>>>, + inverted: bool, +} + +impl DiffHandle { + pub fn new(diff_base: Rope, doc: Rope, redraw_handle: Arc<AtomicBool>) -> DiffHandle { + DiffHandle::new_with_handle(diff_base, doc, redraw_handle).0 + } + + fn new_with_handle( + diff_base: Rope, + doc: Rope, + notify: Arc<AtomicBool>, + ) -> (DiffHandle, JoinHandle<()>) { + let (sender, receiver) = unbounded_channel(); + let hunks: Arc<Mutex<Vec<Hunk>>> = Arc::default(); + let worker = DiffWorker { + channel: receiver, + hunks: hunks.clone(), + new_hunks: Vec::default(), + notify, + }; + let handle = tokio::spawn(worker.run(diff_base, doc)); + let differ = DiffHandle { + channel: sender, + hunks, + inverted: false, + }; + (differ, handle) + } + + pub fn invert(&mut self) { + self.inverted = !self.inverted; + } + + pub fn hunks(&self) -> FileHunks { + FileHunks { + hunks: self.hunks.lock(), + inverted: self.inverted, + } + } + + pub fn update_document(&self, doc: Rope) -> bool { + if self.inverted { + self.update_diff_base_impl(doc) + } else { + self.update_document_impl(doc) + } + } + + pub fn update_diff_base(&self, diff_base: Rope) -> bool { + if self.inverted { + self.update_document_impl(diff_base) + } else { + self.update_diff_base_impl(diff_base) + } + } + + pub fn update_document_impl(&self, doc: Rope) -> bool { + self.channel.send(Event::UpdateDocument(doc)).is_ok() + } + + pub fn update_diff_base_impl(&self, diff_base: Rope) -> bool { + self.channel.send(Event::UpdateDiffBase(diff_base)).is_ok() + } +} + +// TODO configuration +const DIFF_DEBOUNCE_TIME: u64 = 100; +const ALGORITHM: Algorithm = Algorithm::Histogram; +const MAX_DIFF_LINES: usize = u16::MAX as usize; +// cap average line length to 128 for files with MAX_DIFF_LINES +const MAX_DIFF_BYTES: usize = MAX_DIFF_LINES * 128; + +/// A single change in a file potentially sppaning multiple lines +/// Hunks produced by the differs are always ordered by their position +/// in the file and non-overlapping. +/// Specifically for any two hunks `x` and `y` the following properties hold: +/// +/// ``` no_compile +/// assert!(x.before.end <= y.before.start); +/// assert!(x.after.end <= y.after.start); +/// ``` +#[derive(PartialEq, Eq, Clone, Debug)] +pub struct Hunk { + pub before: Range<u32>, + pub after: Range<u32>, +} + +impl Hunk { + /// Can be used instead of `Option::None` for better performance + /// because lines larger then `i32::MAX` are not supported by imara-diff anways. + /// Has some nice properties where it usually is not necessary to check for `None` seperatly: + /// Empty ranges fail contains checks and also faills smaller then checks. + pub const NONE: Hunk = Hunk { + before: u32::MAX..u32::MAX, + after: u32::MAX..u32::MAX, + }; + + /// Inverts a change so that `before` + pub fn invert(&self) -> Hunk { + Hunk { + before: self.after.clone(), + after: self.before.clone(), + } + } + + pub fn is_pure_insertion(&self) -> bool { + self.before.is_empty() + } + + pub fn is_pure_removal(&self) -> bool { + self.after.is_empty() + } +} + +/// A list of changes in a file sorted in ascending +/// non-overlapping order +#[derive(Debug)] +pub struct FileHunks<'a> { + hunks: MutexGuard<'a, Vec<Hunk>>, + inverted: bool, +} + +impl FileHunks<'_> { + pub fn is_inverted(&self) -> bool { + self.inverted + } + + /// Returns the `Hunk` for the `n`th change in this file. + /// if there is no `n`th change `Hunk::NONE` is returned instead. + pub fn nth_hunk(&self, n: u32) -> Hunk { + match self.hunks.get(n as usize) { + Some(hunk) if self.inverted => hunk.invert(), + Some(hunk) => hunk.clone(), + None => Hunk::NONE, + } + } + + pub fn len(&self) -> u32 { + self.hunks.len() as u32 + } + + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} diff --git a/helix-vcs/src/diff/line_cache.rs b/helix-vcs/src/diff/line_cache.rs new file mode 100644 index 000000000000..33c8c45eccd1 --- /dev/null +++ b/helix-vcs/src/diff/line_cache.rs @@ -0,0 +1,130 @@ +//! This modules encapsulates a tiny bit of unsafe code that +//! makes diffing significantly faster and more ergonomic to implement. +//! This code is necessary because diffing requires quick random +//! access to the lines of the text that is being diffed. +//! +//! Therefore it is best to collect the `Rope::lines` iterator into a vec +//! first because access to the vec is `O(1)` where `Rope::line` is `O(log N)`. +//! However this process can allocate a (potentially quite large) vector. +//! +//! To avoid reallocation for every diff, the vector is reused. +//! However the RopeSlice references the original rope and therefore forms a self-referential data structure. +//! A transmute is used to change the lifetime of the slice to static to circumvent that project. +use std::mem::transmute; + +use helix_core::{Rope, RopeSlice}; +use imara_diff::intern::{InternedInput, Interner}; + +use super::{MAX_DIFF_BYTES, MAX_DIFF_LINES}; + +/// A cache that stores the `lines` of a rope as a vector. +/// It allows safely reusing the allocation of the vec when updating the rope +pub(crate) struct InternedRopeLines { + diff_base: Rope, + doc: Rope, + num_tokens_diff_base: u32, + interned: InternedInput<RopeSlice<'static>>, +} + +impl InternedRopeLines { + pub fn new(diff_base: Rope, doc: Rope) -> InternedRopeLines { + let mut res = InternedRopeLines { + interned: InternedInput { + before: Vec::with_capacity(diff_base.len_lines()), + after: Vec::with_capacity(doc.len_lines()), + interner: Interner::new(diff_base.len_lines() + doc.len_lines()), + }, + diff_base, + doc, + // will be populated by update_diff_base_impl + num_tokens_diff_base: 0, + }; + res.update_diff_base_impl(); + res + } + + /// Updates the `diff_base` and optionally the document if `doc` is not None + pub fn update_diff_base(&mut self, diff_base: Rope, doc: Option<Rope>) { + self.interned.clear(); + self.diff_base = diff_base; + if let Some(doc) = doc { + self.doc = doc + } + if !self.is_too_large() { + self.update_diff_base_impl(); + } + } + + /// Updates the `doc` without reintering the `diff_base`, this funciton + /// is therefore significantly faster then `update_diff_base` when only the document changes. + pub fn update_doc(&mut self, doc: Rope) { + // Safety: we clear any tokens that were added after + // the interning of `self.diff_base` finished so + // all lines that refer to `self.doc` have been purged. + + self.interned + .interner + .erase_tokens_after(self.num_tokens_diff_base.into()); + + self.doc = doc; + if self.is_too_large() { + self.interned.after.clear(); + } else { + self.update_doc_impl(); + } + } + + fn update_diff_base_impl(&mut self) { + // Safety: This transmute is save because it only transmutes a lifetime, which has no effect. + // The backing storage for the RopeSlices referred to by the lifetime is stored in `self.diff_base`. + // Therefore as long as `self.diff_base` is not dropped/replaced this memory remains valid. + // `self.diff_base` is only changed in `self.update_diff_base`, which clears the interner. + // When the interned lines are exposed to consumer in `self.diff_input`, the lifetime is bounded to a reference to self. + // That means that on calls to update there exist no references to `self.interned`. + let before = self + .diff_base + .lines() + .map(|line: RopeSlice| -> RopeSlice<'static> { unsafe { transmute(line) } }); + self.interned.update_before(before); + self.num_tokens_diff_base = self.interned.interner.num_tokens(); + // the has to be interned again because the interner was fully cleared + self.update_doc_impl() + } + + fn update_doc_impl(&mut self) { + // Safety: This transmute is save because it only transmutes a lifetime, which has no effect. + // The backing storage for the RopeSlices referred to by the lifetime is stored in `self.doc`. + // Therefore as long as `self.doc` is not dropped/replaced this memory remains valid. + // `self.doc` is only changed in `self.update_doc`, which clears the interner. + // When the interned lines are exposed to consumer in `self.diff_input`, the lifetime is bounded to a reference to self. + // That means that on calls to update there exist no references to `self.interned`. + let after = self + .doc + .lines() + .map(|line: RopeSlice| -> RopeSlice<'static> { unsafe { transmute(line) } }); + self.interned.update_after(after); + } + + fn is_too_large(&self) -> bool { + // bound both lines and bytes to avoid huge files with few (but huge) lines + // or huge file with tiny lines. While this makes no difference to + // diff itself (the diff performance only depends on the number of tokens) + // the interning runtime depends mostly on filesize and is actually dominant + // for large files + self.doc.len_lines() > MAX_DIFF_LINES + || self.diff_base.len_lines() > MAX_DIFF_LINES + || self.doc.len_bytes() > MAX_DIFF_BYTES + || self.diff_base.len_bytes() > MAX_DIFF_BYTES + } + + /// Returns the `InternedInput` for performing the diff. + /// If `diff_base` or `doc` is so large that performing a diff could slow the editor + /// this function returns `None`. + pub fn interned_lines(&self) -> Option<&InternedInput<RopeSlice>> { + if self.is_too_large() { + None + } else { + Some(&self.interned) + } + } +} diff --git a/helix-vcs/src/diff/worker.rs b/helix-vcs/src/diff/worker.rs new file mode 100644 index 000000000000..46936db09b6f --- /dev/null +++ b/helix-vcs/src/diff/worker.rs @@ -0,0 +1,95 @@ +use std::mem::swap; +use std::ops::Range; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +use helix_core::{Rope, RopeSlice}; +use imara_diff::intern::InternedInput; +use parking_lot::Mutex; +use tokio::sync::mpsc::UnboundedReceiver; +use tokio::time::{timeout_at, Duration, Instant}; + +use crate::diff::{Event, ALGORITHM, DIFF_DEBOUNCE_TIME}; + +use super::line_cache::InternedRopeLines; +use super::Hunk; + +#[cfg(test)] +mod test; + +pub(super) struct DiffWorker { + pub channel: UnboundedReceiver<Event>, + pub hunks: Arc<Mutex<Vec<Hunk>>>, + pub new_hunks: Vec<Hunk>, + pub notify: Arc<AtomicBool>, +} + +impl DiffWorker { + pub async fn run(mut self, diff_base: Rope, doc: Rope) { + let mut interner = InternedRopeLines::new(diff_base, doc); + if let Some(lines) = interner.interned_lines() { + self.perform_diff(lines); + } + self.apply_hunks(); + while let Some(event) = self.channel.recv().await { + let mut accumulator = EventAccumulator::new(); + accumulator.handle_event(event); + accumulator + .accumulate_debounced_events(&mut self.channel) + .await; + + if let Some(new_base) = accumulator.diff_base { + interner.update_diff_base(new_base, accumulator.doc) + } else { + interner.update_doc(accumulator.doc.unwrap()) + } + + if let Some(lines) = interner.interned_lines() { + self.perform_diff(lines); + } + self.apply_hunks(); + } + } + + /// update the hunks (used by the gutter) by replacing it with `self.new_hunks`. + /// `self.new_hunks` is always empty after this function runs. + /// To improve performance this function tries to reuse the allocation of the old diff previously stored in `self.line_diffs` + fn apply_hunks(&mut self) { + swap(&mut *self.hunks.lock(), &mut self.new_hunks); + self.notify.store(false, Ordering::Relaxed); + self.new_hunks.clear(); + } + + fn perform_diff(&mut self, input: &InternedInput<RopeSlice>) { + imara_diff::diff(ALGORITHM, input, |before: Range<u32>, after: Range<u32>| { + self.new_hunks.push(Hunk { before, after }) + }) + } +} + +struct EventAccumulator { + diff_base: Option<Rope>, + doc: Option<Rope>, +} + +impl EventAccumulator { + fn new() -> EventAccumulator { + EventAccumulator { + diff_base: None, + doc: None, + } + } + fn handle_event(&mut self, event: Event) { + match event { + Event::UpdateDocument(doc) => self.doc = Some(doc), + Event::UpdateDiffBase(new_diff_base) => self.diff_base = Some(new_diff_base), + } + } + async fn accumulate_debounced_events(&mut self, channel: &mut UnboundedReceiver<Event>) { + let debounce = Duration::from_millis(DIFF_DEBOUNCE_TIME); + let timeout = Instant::now() + debounce; + while let Ok(Some(event)) = timeout_at(timeout, channel.recv()).await { + self.handle_event(event) + } + } +} diff --git a/helix-vcs/src/diff/worker/test.rs b/helix-vcs/src/diff/worker/test.rs new file mode 100644 index 000000000000..d323478ba2c5 --- /dev/null +++ b/helix-vcs/src/diff/worker/test.rs @@ -0,0 +1,152 @@ +use std::sync::atomic::AtomicBool; +use std::sync::Arc; + +use helix_core::Rope; +use tokio::task::JoinHandle; + +use crate::diff::{DiffHandle, Hunk}; + +impl DiffHandle { + fn new_test(diff_base: &str, doc: &str) -> (DiffHandle, JoinHandle<()>) { + DiffHandle::new_with_handle( + Rope::from_str(diff_base), + Rope::from_str(doc), + Arc::new(AtomicBool::new(false)), + ) + } + async fn into_diff(self, handle: JoinHandle<()>) -> Vec<Hunk> { + let hunks = self.hunks; + // dropping the channel terminates the task + drop(self.channel); + handle.await.unwrap(); + let hunks = hunks.lock(); + Vec::clone(&*hunks) + } +} + +#[tokio::test] +async fn append_line() { + let (differ, handle) = DiffHandle::new_test("foo\n", "foo\nbar\n"); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[Hunk { + before: 1..1, + after: 1..2 + }] + ) +} + +#[tokio::test] +async fn prepend_line() { + let (differ, handle) = DiffHandle::new_test("foo\n", "bar\nfoo\n"); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[Hunk { + before: 0..0, + after: 0..1 + }] + ) +} + +#[tokio::test] +async fn modify() { + let (differ, handle) = DiffHandle::new_test("foo\nbar\n", "foo bar\nbar\n"); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[Hunk { + before: 0..1, + after: 0..1 + }] + ) +} + +#[tokio::test] +async fn delete_line() { + let (differ, handle) = DiffHandle::new_test("foo\nfoo bar\nbar\n", "foo\nbar\n"); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[Hunk { + before: 1..2, + after: 1..1 + }] + ) +} + +#[tokio::test] +async fn delete_line_and_modify() { + let (differ, handle) = DiffHandle::new_test("foo\nbar\ntest\nfoo", "foo\ntest\nfoo bar"); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[ + Hunk { + before: 1..2, + after: 1..1 + }, + Hunk { + before: 3..4, + after: 2..3 + }, + ] + ) +} + +#[tokio::test] +async fn add_use() { + let (differ, handle) = DiffHandle::new_test( + "use ropey::Rope;\nuse tokio::task::JoinHandle;\n", + "use ropey::Rope;\nuse ropey::RopeSlice;\nuse tokio::task::JoinHandle;\n", + ); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[Hunk { + before: 1..1, + after: 1..2 + },] + ) +} + +#[tokio::test] +async fn update_document() { + let (differ, handle) = DiffHandle::new_test("foo\nbar\ntest\nfoo", "foo\nbar\ntest\nfoo"); + differ.update_document(Rope::from_str("foo\ntest\nfoo bar")); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[ + Hunk { + before: 1..2, + after: 1..1 + }, + Hunk { + before: 3..4, + after: 2..3 + }, + ] + ) +} + +#[tokio::test] +async fn update_base() { + let (differ, handle) = DiffHandle::new_test("foo\ntest\nfoo bar", "foo\ntest\nfoo bar"); + differ.update_diff_base(Rope::from_str("foo\nbar\ntest\nfoo")); + let line_diffs = differ.into_diff(handle).await; + assert_eq!( + &line_diffs, + &[ + Hunk { + before: 1..2, + after: 1..1 + }, + Hunk { + before: 3..4, + after: 2..3 + }, + ] + ) +} diff --git a/helix-vcs/src/git.rs b/helix-vcs/src/git.rs new file mode 100644 index 000000000000..82b2b5587fc4 --- /dev/null +++ b/helix-vcs/src/git.rs @@ -0,0 +1,80 @@ +use std::path::Path; + +use git::objs::tree::EntryMode; +use git::sec::trust::DefaultForLevel; +use git::{Commit, ObjectId, Repository, ThreadSafeRepository}; +use git_repository as git; + +use crate::DiffProvider; + +#[cfg(test)] +mod test; + +pub struct Git; + +impl Git { + fn open_repo(path: &Path, ceiling_dir: Option<&Path>) -> Option<ThreadSafeRepository> { + // custom open options + let mut git_open_opts_map = git::sec::trust::Mapping::<git::open::Options>::default(); + + // don't use the global git configs (not needed) + let config = git::permissions::Config { + system: false, + git: false, + user: false, + env: true, + includes: true, + git_binary: false, + }; + // change options for config permissions without touching anything else + git_open_opts_map.reduced = git_open_opts_map.reduced.permissions(git::Permissions { + config, + ..git::Permissions::default_for_level(git::sec::Trust::Reduced) + }); + git_open_opts_map.full = git_open_opts_map.full.permissions(git::Permissions { + config, + ..git::Permissions::default_for_level(git::sec::Trust::Full) + }); + + let mut open_options = git::discover::upwards::Options::default(); + if let Some(ceiling_dir) = ceiling_dir { + open_options.ceiling_dirs = vec![ceiling_dir.to_owned()]; + } + + ThreadSafeRepository::discover_with_environment_overrides_opts( + path, + open_options, + git_open_opts_map, + ) + .ok() + } +} + +impl DiffProvider for Git { + fn get_diff_base(&self, file: &Path) -> Option<Vec<u8>> { + debug_assert!(!file.exists() || file.is_file()); + debug_assert!(file.is_absolute()); + + // TODO cache repository lookup + let repo = Git::open_repo(file.parent()?, None)?.to_thread_local(); + let head = repo.head_commit().ok()?; + let file_oid = find_file_in_commit(&repo, &head, file)?; + + let file_object = repo.find_object(file_oid).ok()?; + Some(file_object.detach().data) + } +} + +/// Finds the object that contains the contents of a file at a specific commit. +fn find_file_in_commit(repo: &Repository, commit: &Commit, file: &Path) -> Option<ObjectId> { + let repo_dir = repo.work_dir()?; + let rel_path = file.strip_prefix(repo_dir).ok()?; + let tree = commit.tree().ok()?; + let tree_entry = tree.lookup_entry_by_path(rel_path).ok()??; + match tree_entry.mode() { + // not a file, everything is new, do not show diff + EntryMode::Tree | EntryMode::Commit | EntryMode::Link => None, + // found a file + EntryMode::Blob | EntryMode::BlobExecutable => Some(tree_entry.object_id()), + } +} diff --git a/helix-vcs/src/git/test.rs b/helix-vcs/src/git/test.rs new file mode 100644 index 000000000000..d6e9af08868a --- /dev/null +++ b/helix-vcs/src/git/test.rs @@ -0,0 +1,121 @@ +use std::{fs::File, io::Write, path::Path, process::Command}; + +use tempfile::TempDir; + +use crate::{DiffProvider, Git}; + +fn exec_git_cmd(args: &str, git_dir: &Path) { + let res = Command::new("git") + .arg("-C") + .arg(git_dir) // execute the git command in this directory + .args(args.split_whitespace()) + .env_remove("GIT_DIR") + .env_remove("GIT_ASKPASS") + .env_remove("SSH_ASKPASS") + .env("GIT_TERMINAL_PROMPT", "false") + .env("GIT_AUTHOR_DATE", "2000-01-01 00:00:00 +0000") + .env("GIT_AUTHOR_EMAIL", "author@example.com") + .env("GIT_AUTHOR_NAME", "author") + .env("GIT_COMMITTER_DATE", "2000-01-02 00:00:00 +0000") + .env("GIT_COMMITTER_EMAIL", "committer@example.com") + .env("GIT_COMMITTER_NAME", "committer") + .env("GIT_CONFIG_COUNT", "2") + .env("GIT_CONFIG_KEY_0", "commit.gpgsign") + .env("GIT_CONFIG_VALUE_0", "false") + .env("GIT_CONFIG_KEY_1", "init.defaultBranch") + .env("GIT_CONFIG_VALUE_1", "main") + .output() + .unwrap_or_else(|_| panic!("`git {args}` failed")); + if !res.status.success() { + println!("{}", String::from_utf8_lossy(&res.stdout)); + eprintln!("{}", String::from_utf8_lossy(&res.stderr)); + panic!("`git {args}` failed (see output above)") + } +} + +fn create_commit(repo: &Path, add_modified: bool) { + if add_modified { + exec_git_cmd("add -A", repo); + } + exec_git_cmd("commit -m message", repo); +} + +fn empty_git_repo() -> TempDir { + let tmp = tempfile::tempdir().expect("create temp dir for git testing"); + exec_git_cmd("init", tmp.path()); + exec_git_cmd("config user.email test@helix.org", tmp.path()); + exec_git_cmd("config user.name helix-test", tmp.path()); + tmp +} + +#[test] +fn missing_file() { + let temp_git = empty_git_repo(); + let file = temp_git.path().join("file.txt"); + File::create(&file).unwrap().write_all(b"foo").unwrap(); + + assert_eq!(Git.get_diff_base(&file), None); +} + +#[test] +fn unmodified_file() { + let temp_git = empty_git_repo(); + let file = temp_git.path().join("file.txt"); + let contents = b"foo".as_slice(); + File::create(&file).unwrap().write_all(contents).unwrap(); + create_commit(temp_git.path(), true); + assert_eq!(Git.get_diff_base(&file), Some(Vec::from(contents))); +} + +#[test] +fn modified_file() { + let temp_git = empty_git_repo(); + let file = temp_git.path().join("file.txt"); + let contents = b"foo".as_slice(); + File::create(&file).unwrap().write_all(contents).unwrap(); + create_commit(temp_git.path(), true); + File::create(&file).unwrap().write_all(b"bar").unwrap(); + + assert_eq!(Git.get_diff_base(&file), Some(Vec::from(contents))); +} + +/// Test that `get_file_head` does not return content for a directory. +/// This is important to correctly cover cases where a directory is removed and replaced by a file. +/// If the contents of the directory object were returned a diff between a path and the directory children would be produced. +#[test] +fn directory() { + let temp_git = empty_git_repo(); + let dir = temp_git.path().join("file.txt"); + std::fs::create_dir(&dir).expect(""); + let file = dir.join("file.txt"); + let contents = b"foo".as_slice(); + File::create(&file).unwrap().write_all(contents).unwrap(); + + create_commit(temp_git.path(), true); + + std::fs::remove_dir_all(&dir).unwrap(); + File::create(&dir).unwrap().write_all(b"bar").unwrap(); + assert_eq!(Git.get_diff_base(&dir), None); +} + +/// Test that `get_file_head` does not return content for a symlink. +/// This is important to correctly cover cases where a symlink is removed and replaced by a file. +/// If the contents of the symlink object were returned a diff between a path and the actual file would be produced (bad ui). +#[cfg(any(unix, windows))] +#[test] +fn symlink() { + #[cfg(unix)] + use std::os::unix::fs::symlink; + #[cfg(not(unix))] + use std::os::windows::fs::symlink_file as symlink; + let temp_git = empty_git_repo(); + let file = temp_git.path().join("file.txt"); + let contents = b"foo".as_slice(); + File::create(&file).unwrap().write_all(contents).unwrap(); + let file_link = temp_git.path().join("file_link.txt"); + symlink("file.txt", &file_link).unwrap(); + + create_commit(temp_git.path(), true); + assert_eq!(Git.get_diff_base(&file_link), None); + assert_eq!(Git.get_diff_base(&file), Some(Vec::from(contents))); +} diff --git a/helix-vcs/src/lib.rs b/helix-vcs/src/lib.rs new file mode 100644 index 000000000000..97320d32518f --- /dev/null +++ b/helix-vcs/src/lib.rs @@ -0,0 +1,51 @@ +use std::path::Path; + +#[cfg(feature = "git")] +pub use git::Git; +#[cfg(not(feature = "git"))] +pub use Dummy as Git; + +#[cfg(feature = "git")] +mod git; + +mod diff; + +pub use diff::{DiffHandle, Hunk}; + +pub trait DiffProvider { + /// Returns the data that a diff should be computed against + /// if this provider is used. + /// The data is returned as raw byte without any decoding or encoding performed + /// to ensure all file encodings are handled correctly. + fn get_diff_base(&self, file: &Path) -> Option<Vec<u8>>; +} + +#[doc(hidden)] +pub struct Dummy; +impl DiffProvider for Dummy { + fn get_diff_base(&self, _file: &Path) -> Option<Vec<u8>> { + None + } +} + +pub struct DiffProviderRegistry { + providers: Vec<Box<dyn DiffProvider>>, +} + +impl DiffProviderRegistry { + pub fn get_diff_base(&self, file: &Path) -> Option<Vec<u8>> { + self.providers + .iter() + .find_map(|provider| provider.get_diff_base(file)) + } +} + +impl Default for DiffProviderRegistry { + fn default() -> Self { + // currently only git is supported + // TODO make this configurable when more providers are added + let git: Box<dyn DiffProvider> = Box::new(Git); + let providers = vec![git]; + DiffProviderRegistry { providers } + } +} diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml index a2a88001bffd..13d5da0e583c 100644 --- a/helix-view/Cargo.toml +++ b/helix-view/Cargo.toml @@ -21,6 +21,7 @@ helix-loader = { version = "0.6", path = "../helix-loader" } helix-lsp = { version = "0.6", path = "../helix-lsp" } helix-dap = { version = "0.6", path = "../helix-dap" } crossterm = { version = "0.25", optional = true } +helix-vcs = { version = "0.6", path = "../helix-vcs" } # Conversion traits once_cell = "1.16" @@ -43,6 +44,7 @@ log = "~0.4" which = "4.2" + [target.'cfg(windows)'.dependencies] clipboard-win = { version = "4.4", features = ["std"] } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0eb54f25fa6e..d39e1504fa9e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -3,6 +3,7 @@ use futures_util::future::BoxFuture; use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::Range; +use helix_vcs::{DiffHandle, DiffProviderRegistry}; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::borrow::Cow; @@ -12,6 +13,7 @@ use std::fmt::Display; use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; +use std::sync::atomic::AtomicBool; use std::sync::Arc; use helix_core::{ @@ -133,6 +135,8 @@ pub struct Document { diagnostics: Vec<Diagnostic>, language_server: Option<Arc<helix_lsp::Client>>, + + diff_handle: Option<DiffHandle>, } use std::{fmt, mem}; @@ -371,6 +375,7 @@ impl Document { last_saved_revision: 0, modified_since_accessed: false, language_server: None, + diff_handle: None, } } @@ -624,16 +629,20 @@ impl Document { } /// Reload the document from its path. - pub fn reload(&mut self, view: &mut View) -> Result<(), Error> { + pub fn reload( + &mut self, + view: &mut View, + provider_registry: &DiffProviderRegistry, + redraw_handle: Arc<AtomicBool>, + ) -> Result<(), Error> { let encoding = &self.encoding; - let path = self.path().filter(|path| path.exists()); - - // If there is no path or the path no longer exists. - if path.is_none() { - bail!("can't find file to reload from"); - } + let path = self + .path() + .filter(|path| path.exists()) + .ok_or_else(|| anyhow!("can't find file to reload from"))? + .to_owned(); - let mut file = std::fs::File::open(path.unwrap())?; + let mut file = std::fs::File::open(&path)?; let (rope, ..) = from_reader(&mut file, Some(encoding))?; // Calculate the difference between the buffer and source text, and apply it. @@ -646,6 +655,11 @@ impl Document { self.detect_indent_and_line_ending(); + match provider_registry.get_diff_base(&path) { + Some(diff_base) => self.set_diff_base(diff_base, redraw_handle), + None => self.diff_handle = None, + } + Ok(()) } @@ -828,6 +842,10 @@ impl Document { tokio::spawn(notify); } } + if let Some(diff_handle) = self.diff_handle.as_ref() { + // TODO patchup hunks synchronously to avoid visual artifcats + diff_handle.update_document(self.text.clone()); + } } success } @@ -1039,6 +1057,23 @@ impl Document { server.is_initialized().then(|| server) } + pub fn diff_handle(&self) -> Option<&DiffHandle> { + self.diff_handle.as_ref() + } + + /// Intialize/updates the differ for this document with a new base. + pub fn set_diff_base(&mut self, diff_base: Vec<u8>, redraw_handle: Arc<AtomicBool>) { + if let Ok((diff_base, _)) = from_reader(&mut diff_base.as_slice(), Some(self.encoding)) { + if let Some(differ) = &self.diff_handle { + differ.update_diff_base(diff_base); + return; + } + self.diff_handle = Some(DiffHandle::new(diff_base, self.text.clone(), redraw_handle)) + } else { + self.diff_handle = None; + } + } + #[inline] /// Tree-sitter AST tree pub fn syntax(&self) -> Option<&Syntax> { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 6eaa89aac814..912691e85c17 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -9,6 +9,7 @@ use crate::{ tree::{self, Tree}, Align, Document, DocumentId, View, ViewId, }; +use helix_vcs::DiffProviderRegistry; use futures_util::stream::select_all::SelectAll; use futures_util::{future, StreamExt}; @@ -22,7 +23,7 @@ use std::{ num::NonZeroUsize, path::{Path, PathBuf}, pin::Pin, - sync::Arc, + sync::{atomic::AtomicBool, Arc}, }; use tokio::{ @@ -454,6 +455,8 @@ pub enum GutterType { LineNumbers, /// Show one blank space Spacer, + /// Highlight local changes + Diff, } impl std::str::FromStr for GutterType { @@ -464,6 +467,7 @@ impl std::str::FromStr for GutterType { "diagnostics" => Ok(Self::Diagnostics), "spacer" => Ok(Self::Spacer), "line-numbers" => Ok(Self::LineNumbers), + "diff" => Ok(Self::Diff), _ => anyhow::bail!("Gutter type can only be `diagnostics` or `line-numbers`."), } } @@ -600,6 +604,8 @@ impl Default for Config { GutterType::Diagnostics, GutterType::Spacer, GutterType::LineNumbers, + GutterType::Spacer, + GutterType::Diff, ], middle_click_paste: true, auto_pairs: AutoPairConfig::default(), @@ -681,6 +687,7 @@ pub struct Editor { pub macro_replaying: Vec<char>, pub language_servers: helix_lsp::Registry, pub diagnostics: BTreeMap<lsp::Url, Vec<lsp::Diagnostic>>, + pub diff_providers: DiffProviderRegistry, pub debugger: Option<dap::Client>, pub debugger_events: SelectAll<UnboundedReceiverStream<dap::Payload>>, @@ -711,6 +718,7 @@ pub struct Editor { pub exit_code: i32, pub config_events: (UnboundedSender<ConfigEvent>, UnboundedReceiver<ConfigEvent>), + pub redraw_handle: Arc<AtomicBool>, } #[derive(Debug)] @@ -785,6 +793,7 @@ impl Editor { theme: theme_loader.default(), language_servers: helix_lsp::Registry::new(), diagnostics: BTreeMap::new(), + diff_providers: DiffProviderRegistry::default(), debugger: None, debugger_events: SelectAll::new(), breakpoints: HashMap::new(), @@ -803,6 +812,7 @@ impl Editor { auto_pairs, exit_code: 0, config_events: unbounded_channel(), + redraw_handle: Arc::new(AtomicBool::new(false)), } } @@ -1107,7 +1117,9 @@ impl Editor { let mut doc = Document::open(&path, None, Some(self.syn_loader.clone()))?; let _ = Self::launch_language_server(&mut self.language_servers, &mut doc); - + if let Some(diff_base) = self.diff_providers.get_diff_base(&path) { + doc.set_diff_base(diff_base, self.redraw_handle.clone()); + } self.new_document(doc) }; @@ -1340,24 +1352,29 @@ impl Editor { } pub async fn wait_event(&mut self) -> EditorEvent { - tokio::select! { - biased; + // the loop only runs once or twice and would be better implemented with a recursion + const generic + // however due to limitations with async functions that can not be implemented right now + loop { + tokio::select! { + biased; + + Some(event) = self.save_queue.next() => { + self.write_count -= 1; + return EditorEvent::DocumentSaved(event) + } + Some(config_event) = self.config_events.1.recv() => { + return EditorEvent::ConfigEvent(config_event) + } + Some(message) = self.language_servers.incoming.next() => { + return EditorEvent::LanguageServerMessage(message) + } + Some(event) = self.debugger_events.next() => { + return EditorEvent::DebuggerEvent(event) + } - Some(event) = self.save_queue.next() => { - self.write_count -= 1; - EditorEvent::DocumentSaved(event) - } - Some(config_event) = self.config_events.1.recv() => { - EditorEvent::ConfigEvent(config_event) - } - Some(message) = self.language_servers.incoming.next() => { - EditorEvent::LanguageServerMessage(message) - } - Some(event) = self.debugger_events.next() => { - EditorEvent::DebuggerEvent(event) - } - _ = &mut self.idle_timer => { - EditorEvent::IdleTimer + _ = &mut self.idle_timer => { + return EditorEvent::IdleTimer + } } } } diff --git a/helix-view/src/gutter.rs b/helix-view/src/gutter.rs index 61a1779186c3..c1de8054e54d 100644 --- a/helix-view/src/gutter.rs +++ b/helix-view/src/gutter.rs @@ -12,7 +12,7 @@ fn count_digits(n: usize) -> usize { std::iter::successors(Some(n), |&n| (n >= 10).then(|| n / 10)).count() } -pub type GutterFn<'doc> = Box<dyn Fn(usize, bool, &mut String) -> Option<Style> + 'doc>; +pub type GutterFn<'doc> = Box<dyn FnMut(usize, bool, &mut String) -> Option<Style> + 'doc>; pub type Gutter = for<'doc> fn(&'doc Editor, &'doc Document, &View, &Theme, bool, usize) -> GutterFn<'doc>; @@ -31,6 +31,7 @@ impl GutterType { } GutterType::LineNumbers => line_numbers(editor, doc, view, theme, is_focused), GutterType::Spacer => padding(editor, doc, view, theme, is_focused), + GutterType::Diff => diff(editor, doc, view, theme, is_focused), } } @@ -39,6 +40,7 @@ impl GutterType { GutterType::Diagnostics => 1, GutterType::LineNumbers => line_numbers_width(_view, doc), GutterType::Spacer => 1, + GutterType::Diff => 1, } } } @@ -83,6 +85,53 @@ pub fn diagnostic<'doc>( }) } +pub fn diff<'doc>( + _editor: &'doc Editor, + doc: &'doc Document, + _view: &View, + theme: &Theme, + _is_focused: bool, +) -> GutterFn<'doc> { + let added = theme.get("diff.plus"); + let deleted = theme.get("diff.minus"); + let modified = theme.get("diff.delta"); + if let Some(diff_handle) = doc.diff_handle() { + let hunks = diff_handle.hunks(); + let mut hunk_i = 0; + let mut hunk = hunks.nth_hunk(hunk_i); + Box::new(move |line: usize, _selected: bool, out: &mut String| { + // truncating the line is fine here because we don't compute diffs + // for files with more lines then i32::MAX anyways + // we need to special case removals here + // these technically do not have a range of lines to highlight (`hunk.after.start == hunk.after.end`). + // However we still want to display these hunks correctly we must not yet scipe to the next hunk here + while hunk.after.end < line as u32 + || !hunk.is_pure_removal() && line as u32 == hunk.after.end + { + hunk_i += 1; + hunk = hunks.nth_hunk(hunk_i); + } + + if hunk.after.start > line as u32 { + return None; + } + + let (icon, style) = if hunk.is_pure_insertion() { + ("▍", added) + } else if hunk.is_pure_removal() { + ("▔", deleted) + } else { + ("▍", modified) + }; + + write!(out, "{}", icon).unwrap(); + Some(style) + }) + } else { + Box::new(move |_, _, _| None) + } +} + pub fn line_numbers<'doc>( editor: &'doc Editor, doc: &'doc Document, @@ -226,8 +275,8 @@ pub fn diagnostics_or_breakpoints<'doc>( theme: &Theme, is_focused: bool, ) -> GutterFn<'doc> { - let diagnostics = diagnostic(editor, doc, view, theme, is_focused); - let breakpoints = breakpoints(editor, doc, view, theme, is_focused); + let mut diagnostics = diagnostic(editor, doc, view, theme, is_focused); + let mut breakpoints = breakpoints(editor, doc, view, theme, is_focused); Box::new(move |line, selected, out| { breakpoints(line, selected, out).or_else(|| diagnostics(line, selected, out)) diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index c917a1abc054..8e3e457cd4a8 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -149,17 +149,10 @@ impl View { } pub fn gutter_offset(&self, doc: &Document) -> u16 { - let mut offset = self - .gutters + self.gutters .iter() .map(|gutter| gutter.width(self, doc) as u16) - .sum(); - - if offset > 0 { - offset += 1 - } - - offset + .sum() } // @@ -360,8 +353,8 @@ impl View { mod tests { use super::*; use helix_core::Rope; - const OFFSET: u16 = 4; // 1 diagnostic + 2 linenr (< 100 lines) + 1 gutter - const OFFSET_WITHOUT_LINE_NUMBERS: u16 = 2; // 1 diagnostic + 1 gutter + const OFFSET: u16 = 3; // 1 diagnostic + 2 linenr (< 100 lines) + const OFFSET_WITHOUT_LINE_NUMBERS: u16 = 1; // 1 diagnostic // const OFFSET: u16 = GUTTERS.iter().map(|(_, width)| *width as u16).sum(); use crate::document::Document; use crate::editor::GutterType; From 6e58077f651bcd371c9c9aa9279fc7a1b00ef731 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe <pascal.kuthe@semimod.de> Date: Sat, 19 Nov 2022 23:35:45 +0100 Subject: [PATCH 2/5] update diff gutter synchronously, with a timeout --- helix-term/src/application.rs | 52 ++++++---- helix-vcs/Cargo.toml | 10 +- helix-vcs/src/diff.rs | 66 +++++++----- helix-vcs/src/diff/worker.rs | 163 ++++++++++++++++++++++++------ helix-vcs/src/diff/worker/test.rs | 5 +- helix-view/src/document.rs | 15 +-- helix-view/src/editor.rs | 42 +++++++- 7 files changed, 260 insertions(+), 93 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index ec377689e487..b62d8713b082 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -31,7 +31,7 @@ use crate::{ use log::{debug, error, warn}; use std::{ io::{stdin, stdout, Write}, - sync::{atomic::Ordering, Arc}, + sync::Arc, time::{Duration, Instant}, }; @@ -274,17 +274,26 @@ impl Application { } #[cfg(feature = "integration")] - fn render(&mut self) {} + async fn render(&mut self) {} #[cfg(not(feature = "integration"))] - fn render(&mut self) { + async fn render(&mut self) { let mut cx = crate::compositor::Context { editor: &mut self.editor, jobs: &mut self.jobs, scroll: None, }; - cx.editor.redraw_handle.store(false, Ordering::Relaxed); + // Aquire mutable access to the redraw_handle lock + // to ensure that there are no tasks running that want to block rendering + drop(cx.editor.redraw_handle.1.write().await); + cx.editor.needs_redraw = false; + { + // exhaust any leftover redraw notifications + let notify = cx.editor.redraw_handle.0.notified(); + tokio::pin!(notify); + notify.enable(); + } let area = self .terminal @@ -306,7 +315,7 @@ impl Application { where S: Stream<Item = crossterm::Result<crossterm::event::Event>> + Unpin, { - self.render(); + self.render().await; self.last_render = Instant::now(); loop { @@ -331,18 +340,18 @@ impl Application { biased; Some(event) = input_stream.next() => { - self.handle_terminal_events(event); + self.handle_terminal_events(event).await; } Some(signal) = self.signals.next() => { self.handle_signals(signal).await; } Some(callback) = self.jobs.futures.next() => { self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback); - self.render(); + self.render().await; } Some(callback) = self.jobs.wait_futures.next() => { self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback); - self.render(); + self.render().await; } event = self.editor.wait_event() => { let _idle_handled = self.handle_editor_event(event).await; @@ -447,25 +456,25 @@ impl Application { self.compositor.resize(area); self.terminal.clear().expect("couldn't clear terminal"); - self.render(); + self.render().await; } signal::SIGUSR1 => { self.refresh_config(); - self.render(); + self.render().await; } _ => unreachable!(), } } - pub fn handle_idle_timeout(&mut self) { + pub async fn handle_idle_timeout(&mut self) { let mut cx = crate::compositor::Context { editor: &mut self.editor, jobs: &mut self.jobs, scroll: None, }; let should_render = self.compositor.handle_event(&Event::IdleTimeout, &mut cx); - if should_render || self.editor.redraw_handle.load(Ordering::Relaxed) { - self.render(); + if should_render || self.editor.needs_redraw { + self.render().await; } } @@ -538,11 +547,11 @@ impl Application { match event { EditorEvent::DocumentSaved(event) => { self.handle_document_write(event); - self.render(); + self.render().await; } EditorEvent::ConfigEvent(event) => { self.handle_config_events(event); - self.render(); + self.render().await; } EditorEvent::LanguageServerMessage((id, call)) => { self.handle_language_server_message(call, id).await; @@ -550,19 +559,19 @@ impl Application { let last = self.editor.language_servers.incoming.is_empty(); if last || self.last_render.elapsed() > LSP_DEADLINE { - self.render(); + self.render().await; self.last_render = Instant::now(); } } EditorEvent::DebuggerEvent(payload) => { let needs_render = self.editor.handle_debugger_message(payload).await; if needs_render { - self.render(); + self.render().await; } } EditorEvent::IdleTimer => { self.editor.clear_idle_timer(); - self.handle_idle_timeout(); + self.handle_idle_timeout().await; #[cfg(feature = "integration")] { @@ -574,7 +583,10 @@ impl Application { false } - pub fn handle_terminal_events(&mut self, event: Result<CrosstermEvent, crossterm::ErrorKind>) { + pub async fn handle_terminal_events( + &mut self, + event: Result<CrosstermEvent, crossterm::ErrorKind>, + ) { let mut cx = crate::compositor::Context { editor: &mut self.editor, jobs: &mut self.jobs, @@ -598,7 +610,7 @@ impl Application { }; if should_redraw && !self.editor.should_close() { - self.render(); + self.render().await; } } diff --git a/helix-vcs/Cargo.toml b/helix-vcs/Cargo.toml index f2b1a3f7e416..22ae0be5131c 100644 --- a/helix-vcs/Cargo.toml +++ b/helix-vcs/Cargo.toml @@ -11,12 +11,16 @@ homepage = "https://helix-editor.com" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -git-repository = { version = "0.26", default-features = false , optional = true } + +helix-core = { version = "0.6", path = "../helix-core" } + tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "parking_lot", "macros"] } +parking_lot = "0.12" + +git-repository = { version = "0.26", default-features = false , optional = true } imara-diff = "0.1.5" -helix-core = { version = "0.6", path = "../helix-core" } + log = "0.4" -parking_lot = "0.12" [features] git = ["git-repository"] diff --git a/helix-vcs/src/diff.rs b/helix-vcs/src/diff.rs index 084c725f58e4..e02c51d1b52f 100644 --- a/helix-vcs/src/diff.rs +++ b/helix-vcs/src/diff.rs @@ -1,11 +1,11 @@ use std::ops::Range; -use std::sync::atomic::AtomicBool; use std::sync::Arc; use helix_core::Rope; use imara_diff::Algorithm; use parking_lot::{Mutex, MutexGuard}; use tokio::sync::mpsc::{unbounded_channel, UnboundedSender}; +use tokio::sync::{Notify, RwLock}; use tokio::task::JoinHandle; use crate::diff::worker::DiffWorker; @@ -13,9 +13,21 @@ use crate::diff::worker::DiffWorker; mod line_cache; mod worker; -enum Event { - UpdateDocument(Rope), - UpdateDiffBase(Rope), +type RedrawHandle = Arc<(Notify, RwLock<()>)>; + +// The order of enum variants is used by the PartialOrd +// derive macro, DO NOT REORDER +#[derive(PartialEq, PartialOrd)] +enum RenderStrategy { + Async, + SyncWithTimeout, + Sync, +} + +struct Event { + text: Rope, + is_base: bool, + render_strategy: RenderStrategy, } #[derive(Clone, Debug)] @@ -26,14 +38,14 @@ pub struct DiffHandle { } impl DiffHandle { - pub fn new(diff_base: Rope, doc: Rope, redraw_handle: Arc<AtomicBool>) -> DiffHandle { + pub fn new(diff_base: Rope, doc: Rope, redraw_handle: RedrawHandle) -> DiffHandle { DiffHandle::new_with_handle(diff_base, doc, redraw_handle).0 } fn new_with_handle( diff_base: Rope, doc: Rope, - notify: Arc<AtomicBool>, + redraw_handle: RedrawHandle, ) -> (DiffHandle, JoinHandle<()>) { let (sender, receiver) = unbounded_channel(); let hunks: Arc<Mutex<Vec<Hunk>>> = Arc::default(); @@ -41,7 +53,8 @@ impl DiffHandle { channel: receiver, hunks: hunks.clone(), new_hunks: Vec::default(), - notify, + redraw_handle, + difff_finished_notify: Arc::default(), }; let handle = tokio::spawn(worker.run(diff_base, doc)); let differ = DiffHandle { @@ -63,35 +76,38 @@ impl DiffHandle { } } - pub fn update_document(&self, doc: Rope) -> bool { - if self.inverted { - self.update_diff_base_impl(doc) + pub fn update_document(&self, doc: Rope, block: bool) -> bool { + let mode = if block { + RenderStrategy::Sync } else { - self.update_document_impl(doc) - } + RenderStrategy::SyncWithTimeout + }; + self.update_document_impl(doc, self.inverted, mode) } pub fn update_diff_base(&self, diff_base: Rope) -> bool { - if self.inverted { - self.update_document_impl(diff_base) - } else { - self.update_diff_base_impl(diff_base) - } + self.update_document_impl(diff_base, !self.inverted, RenderStrategy::Async) } - pub fn update_document_impl(&self, doc: Rope) -> bool { - self.channel.send(Event::UpdateDocument(doc)).is_ok() - } - - pub fn update_diff_base_impl(&self, diff_base: Rope) -> bool { - self.channel.send(Event::UpdateDiffBase(diff_base)).is_ok() + fn update_document_impl(&self, text: Rope, is_base: bool, mode: RenderStrategy) -> bool { + let event = Event { + text, + is_base, + render_strategy: mode, + }; + self.channel.send(event).is_ok() } } // TODO configuration -const DIFF_DEBOUNCE_TIME: u64 = 100; +/// synchronus debounce value should be low +/// so we can update synchrously most of the time +const DIFF_DEBOUNCE_TIME_SYNC: u64 = 1; +/// maximum time that rendering should be blocked until the diff finishes +const SYNC_DIFF_TIMEOUT: u64 = 50; +const DIFF_DEBOUNCE_TIME_ASYNC: u64 = 100; const ALGORITHM: Algorithm = Algorithm::Histogram; -const MAX_DIFF_LINES: usize = u16::MAX as usize; +const MAX_DIFF_LINES: usize = 64 * u16::MAX as usize; // cap average line length to 128 for files with MAX_DIFF_LINES const MAX_DIFF_BYTES: usize = MAX_DIFF_LINES * 128; diff --git a/helix-vcs/src/diff/worker.rs b/helix-vcs/src/diff/worker.rs index 46936db09b6f..b83bf471d8f2 100644 --- a/helix-vcs/src/diff/worker.rs +++ b/helix-vcs/src/diff/worker.rs @@ -1,15 +1,18 @@ use std::mem::swap; use std::ops::Range; -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use helix_core::{Rope, RopeSlice}; use imara_diff::intern::InternedInput; use parking_lot::Mutex; use tokio::sync::mpsc::UnboundedReceiver; -use tokio::time::{timeout_at, Duration, Instant}; +use tokio::sync::{Notify, RwLockReadGuard}; +use tokio::time::{timeout, timeout_at, Duration, Instant}; -use crate::diff::{Event, ALGORITHM, DIFF_DEBOUNCE_TIME}; +use crate::diff::{ + Event, RedrawHandle, RenderStrategy, ALGORITHM, DIFF_DEBOUNCE_TIME_ASYNC, + DIFF_DEBOUNCE_TIME_SYNC, SYNC_DIFF_TIMEOUT, +}; use super::line_cache::InternedRopeLines; use super::Hunk; @@ -21,10 +24,24 @@ pub(super) struct DiffWorker { pub channel: UnboundedReceiver<Event>, pub hunks: Arc<Mutex<Vec<Hunk>>>, pub new_hunks: Vec<Hunk>, - pub notify: Arc<AtomicBool>, + pub redraw_handle: RedrawHandle, + pub difff_finished_notify: Arc<Notify>, } impl DiffWorker { + async fn accumulate_events(&mut self, event: Event) -> (Option<Rope>, Option<Rope>) { + let mut accumulator = EventAccumulator::new(&self.redraw_handle); + accumulator.handle_event(event).await; + accumulator + .accumulate_debounced_events( + &mut self.channel, + self.redraw_handle.clone(), + self.difff_finished_notify.clone(), + ) + .await; + (accumulator.doc, accumulator.diff_base) + } + pub async fn run(mut self, diff_base: Rope, doc: Rope) { let mut interner = InternedRopeLines::new(diff_base, doc); if let Some(lines) = interner.interned_lines() { @@ -32,21 +49,28 @@ impl DiffWorker { } self.apply_hunks(); while let Some(event) = self.channel.recv().await { - let mut accumulator = EventAccumulator::new(); - accumulator.handle_event(event); - accumulator - .accumulate_debounced_events(&mut self.channel) - .await; - - if let Some(new_base) = accumulator.diff_base { - interner.update_diff_base(new_base, accumulator.doc) - } else { - interner.update_doc(accumulator.doc.unwrap()) - } + let (doc, diff_base) = self.accumulate_events(event).await; + + let process_accumulated_events = || { + if let Some(new_base) = diff_base { + interner.update_diff_base(new_base, doc) + } else { + interner.update_doc(doc.unwrap()) + } + + if let Some(lines) = interner.interned_lines() { + self.perform_diff(lines) + } + }; + + // Calculating diffs is computationally expensive and should + // not run inside an async function to avoid blocking other futures. + // Note: tokio::task::block_in_place does not work during tests + #[cfg(test)] + process_accumulated_events(); + #[cfg(not(test))] + tokio::task::block_in_place(process_accumulated_events); - if let Some(lines) = interner.interned_lines() { - self.perform_diff(lines); - } self.apply_hunks(); } } @@ -56,7 +80,7 @@ impl DiffWorker { /// To improve performance this function tries to reuse the allocation of the old diff previously stored in `self.line_diffs` fn apply_hunks(&mut self) { swap(&mut *self.hunks.lock(), &mut self.new_hunks); - self.notify.store(false, Ordering::Relaxed); + self.difff_finished_notify.notify_waiters(); self.new_hunks.clear(); } @@ -67,29 +91,106 @@ impl DiffWorker { } } -struct EventAccumulator { +struct EventAccumulator<'a> { diff_base: Option<Rope>, doc: Option<Rope>, + render_stratagey: RenderStrategy, + redraw_handle: &'a RedrawHandle, + render_lock: Option<RwLockReadGuard<'a, ()>>, + timeout: Instant, } -impl EventAccumulator { - fn new() -> EventAccumulator { +impl<'a> EventAccumulator<'a> { + fn new(redraw_handle: &'a RedrawHandle) -> EventAccumulator<'a> { EventAccumulator { diff_base: None, doc: None, + render_stratagey: RenderStrategy::Async, + render_lock: None, + redraw_handle, + timeout: Instant::now(), } } - fn handle_event(&mut self, event: Event) { - match event { - Event::UpdateDocument(doc) => self.doc = Some(doc), - Event::UpdateDiffBase(new_diff_base) => self.diff_base = Some(new_diff_base), + + async fn handle_event(&mut self, event: Event) { + let dst = if event.is_base { + &mut self.diff_base + } else { + &mut self.doc + }; + + *dst = Some(event.text); + + // always prefer the most synchronus requested render mode + if event.render_strategy > self.render_stratagey { + if self.render_lock.is_none() { + self.timeout = Instant::now() + Duration::from_millis(SYNC_DIFF_TIMEOUT); + self.render_lock = Some(self.redraw_handle.1.read().await); + } + self.render_stratagey = event.render_strategy } } - async fn accumulate_debounced_events(&mut self, channel: &mut UnboundedReceiver<Event>) { - let debounce = Duration::from_millis(DIFF_DEBOUNCE_TIME); - let timeout = Instant::now() + debounce; - while let Ok(Some(event)) = timeout_at(timeout, channel.recv()).await { - self.handle_event(event) + + async fn accumulate_debounced_events( + &mut self, + channel: &mut UnboundedReceiver<Event>, + redraw_handle: RedrawHandle, + diff_finished_notify: Arc<Notify>, + ) { + let async_debounce = Duration::from_millis(DIFF_DEBOUNCE_TIME_ASYNC); + let sync_debounce = Duration::from_millis(DIFF_DEBOUNCE_TIME_SYNC); + loop { + let debounce = if self.render_stratagey == RenderStrategy::Async { + async_debounce + } else { + sync_debounce + }; + + if let Ok(Some(event)) = timeout(debounce, channel.recv()).await { + self.handle_event(event).await; + } else { + break; + } } + + // setup task to trigger the rendering + // with the choosen render stragey + match self.render_stratagey { + RenderStrategy::Async => { + tokio::spawn(async move { + diff_finished_notify.notified().await; + redraw_handle.0.notify_one(); + }); + } + RenderStrategy::SyncWithTimeout => { + let timeout = self.timeout; + tokio::spawn(async move { + let res = { + // Aquire a lock on the redraw handle. + // The lock will block the rendering from occuring while held. + // The rendering waits for the diff if it doesn't time out + let _render_guard = redraw_handle.1.read(); + timeout_at(timeout, diff_finished_notify.notified()).await + }; + if res.is_ok() { + // Diff finished in time we are done. + return; + } + // Diff failed to complete in time log the event + // and wait until the diff occurs to trigger an async redraw + log::warn!("Diff computation timed out, update of diffs might appear delayed"); + diff_finished_notify.notified().await; + redraw_handle.0.notify_one(); + }); + } + RenderStrategy::Sync => { + tokio::spawn(async move { + // Aquire a lock on the redraw handle. + // The lock will block the rendering from occuring while held. + let _render_guard = redraw_handle.1.read(); + diff_finished_notify.notified().await + }); + } + }; } } diff --git a/helix-vcs/src/diff/worker/test.rs b/helix-vcs/src/diff/worker/test.rs index d323478ba2c5..88dea9c7de5f 100644 --- a/helix-vcs/src/diff/worker/test.rs +++ b/helix-vcs/src/diff/worker/test.rs @@ -1,4 +1,3 @@ -use std::sync::atomic::AtomicBool; use std::sync::Arc; use helix_core::Rope; @@ -11,7 +10,7 @@ impl DiffHandle { DiffHandle::new_with_handle( Rope::from_str(diff_base), Rope::from_str(doc), - Arc::new(AtomicBool::new(false)), + Arc::default(), ) } async fn into_diff(self, handle: JoinHandle<()>) -> Vec<Hunk> { @@ -114,7 +113,7 @@ async fn add_use() { #[tokio::test] async fn update_document() { let (differ, handle) = DiffHandle::new_test("foo\nbar\ntest\nfoo", "foo\nbar\ntest\nfoo"); - differ.update_document(Rope::from_str("foo\ntest\nfoo bar")); + differ.update_document(Rope::from_str("foo\ntest\nfoo bar"), false); let line_diffs = differ.into_diff(handle).await; assert_eq!( &line_diffs, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index d39e1504fa9e..7eadb2593987 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -4,6 +4,7 @@ use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::Range; use helix_vcs::{DiffHandle, DiffProviderRegistry}; + use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::borrow::Cow; @@ -13,7 +14,6 @@ use std::fmt::Display; use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; -use std::sync::atomic::AtomicBool; use std::sync::Arc; use helix_core::{ @@ -26,6 +26,7 @@ use helix_core::{ DEFAULT_LINE_ENDING, }; +use crate::editor::RedrawHandle; use crate::{apply_transaction, DocumentId, Editor, View, ViewId}; /// 8kB of buffer space for encoding and decoding `Rope`s. @@ -633,7 +634,7 @@ impl Document { &mut self, view: &mut View, provider_registry: &DiffProviderRegistry, - redraw_handle: Arc<AtomicBool>, + redraw_handle: RedrawHandle, ) -> Result<(), Error> { let encoding = &self.encoding; let path = self @@ -801,6 +802,10 @@ impl Document { if !transaction.changes().is_empty() { self.version += 1; + // start computing the diff in parallel + if let Some(diff_handle) = &self.diff_handle { + diff_handle.update_document(self.text.clone(), false); + } // generate revert to savepoint if self.savepoint.is_some() { @@ -842,10 +847,6 @@ impl Document { tokio::spawn(notify); } } - if let Some(diff_handle) = self.diff_handle.as_ref() { - // TODO patchup hunks synchronously to avoid visual artifcats - diff_handle.update_document(self.text.clone()); - } } success } @@ -1062,7 +1063,7 @@ impl Document { } /// Intialize/updates the differ for this document with a new base. - pub fn set_diff_base(&mut self, diff_base: Vec<u8>, redraw_handle: Arc<AtomicBool>) { + pub fn set_diff_base(&mut self, diff_base: Vec<u8>, redraw_handle: RedrawHandle) { if let Ok((diff_base, _)) = from_reader(&mut diff_base.as_slice(), Some(self.encoding)) { if let Some(differ) = &self.diff_handle { differ.update_diff_base(diff_base); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 912691e85c17..dd4ea76a8f88 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -23,11 +23,14 @@ use std::{ num::NonZeroUsize, path::{Path, PathBuf}, pin::Pin, - sync::{atomic::AtomicBool, Arc}, + sync::Arc, }; use tokio::{ - sync::mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender}, + sync::{ + mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender}, + Notify, RwLock, + }, time::{sleep, Duration, Instant, Sleep}, }; @@ -150,6 +153,14 @@ pub struct Config { deserialize_with = "deserialize_duration_millis" )] pub idle_timeout: Duration, + /// Time in milliseconds since last keypress before a redraws trigger. + /// Used for redrawing asynchronsouly computed UI compoenents, set to 0 for instant. + /// Defaults to 100ms. + #[serde( + serialize_with = "serialize_duration_millis", + deserialize_with = "deserialize_duration_millis" + )] + pub redraw_timeout: Duration, pub completion_trigger_len: u8, /// Whether to display infoboxes. Defaults to true. pub auto_info: bool, @@ -627,6 +638,7 @@ impl Default for Config { bufferline: BufferLine::default(), indent_guides: IndentGuidesConfig::default(), color_modes: false, + redraw_timeout: Duration::from_millis(200), } } } @@ -718,9 +730,15 @@ pub struct Editor { pub exit_code: i32, pub config_events: (UnboundedSender<ConfigEvent>, UnboundedReceiver<ConfigEvent>), - pub redraw_handle: Arc<AtomicBool>, + /// Allows asynchronous tasks to control the rendering + /// The `Notify` allows asynchronous tasks to request the editor to perform a redraw + /// The `RwLock` blocks the editor from performing the render until an exclusive lock can be aquired + pub redraw_handle: Arc<(Notify, RwLock<()>)>, + pub needs_redraw: bool, } +pub type RedrawHandle = Arc<(Notify, RwLock<()>)>; + #[derive(Debug)] pub enum EditorEvent { DocumentSaved(DocumentSavedEventResult), @@ -812,7 +830,8 @@ impl Editor { auto_pairs, exit_code: 0, config_events: unbounded_channel(), - redraw_handle: Arc::new(AtomicBool::new(false)), + redraw_handle: Arc::default(), + needs_redraw: false, } } @@ -847,6 +866,11 @@ impl Editor { .reset(Instant::now() + config.idle_timeout); } + fn redraw_deadline(&self) -> Instant { + let config = self.config(); + Instant::now() + config.redraw_timeout + } + pub fn clear_status(&mut self) { self.status_msg = None; } @@ -1372,6 +1396,16 @@ impl Editor { return EditorEvent::DebuggerEvent(event) } + _ = self.redraw_handle.0.notified() => { + if !self.needs_redraw{ + self.needs_redraw = true; + let timeout = self.redraw_deadline(); + if timeout < self.idle_timer.deadline(){ + self.idle_timer.as_mut().reset(timeout) + } + } + } + _ = &mut self.idle_timer => { return EditorEvent::IdleTimer } From f38829ac3acf9edfb6f406c27bddc05c5d9a22d4 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe <pascal.kuthe@semimod.de> Date: Mon, 28 Nov 2022 04:00:26 +0100 Subject: [PATCH 3/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Blaž Hrastnik <blaz@mxxn.io> Co-authored-by: Michael Davis <mcarsondavis@gmail.com> --- helix-vcs/Cargo.toml | 1 - helix-vcs/src/diff.rs | 12 ++++++------ helix-vcs/src/diff/line_cache.rs | 6 +++--- helix-vcs/src/diff/worker.rs | 8 ++++---- helix-view/src/editor.rs | 2 +- helix-view/src/gutter.rs | 4 ++-- 6 files changed, 16 insertions(+), 17 deletions(-) diff --git a/helix-vcs/Cargo.toml b/helix-vcs/Cargo.toml index 22ae0be5131c..c114666d26d5 100644 --- a/helix-vcs/Cargo.toml +++ b/helix-vcs/Cargo.toml @@ -11,7 +11,6 @@ homepage = "https://helix-editor.com" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] - helix-core = { version = "0.6", path = "../helix-core" } tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "parking_lot", "macros"] } diff --git a/helix-vcs/src/diff.rs b/helix-vcs/src/diff.rs index e02c51d1b52f..240b30f646e1 100644 --- a/helix-vcs/src/diff.rs +++ b/helix-vcs/src/diff.rs @@ -100,8 +100,8 @@ impl DiffHandle { } // TODO configuration -/// synchronus debounce value should be low -/// so we can update synchrously most of the time +/// synchronous debounce value should be low +/// so we can update synchronously most of the time const DIFF_DEBOUNCE_TIME_SYNC: u64 = 1; /// maximum time that rendering should be blocked until the diff finishes const SYNC_DIFF_TIMEOUT: u64 = 50; @@ -111,7 +111,7 @@ const MAX_DIFF_LINES: usize = 64 * u16::MAX as usize; // cap average line length to 128 for files with MAX_DIFF_LINES const MAX_DIFF_BYTES: usize = MAX_DIFF_LINES * 128; -/// A single change in a file potentially sppaning multiple lines +/// A single change in a file potentially spanning multiple lines /// Hunks produced by the differs are always ordered by their position /// in the file and non-overlapping. /// Specifically for any two hunks `x` and `y` the following properties hold: @@ -128,15 +128,15 @@ pub struct Hunk { impl Hunk { /// Can be used instead of `Option::None` for better performance - /// because lines larger then `i32::MAX` are not supported by imara-diff anways. + /// because lines larger than `i32::MAX` are not supported by imara-diff anways. /// Has some nice properties where it usually is not necessary to check for `None` seperatly: - /// Empty ranges fail contains checks and also faills smaller then checks. + /// Empty ranges fail contains checks and also fails smaller than checks. pub const NONE: Hunk = Hunk { before: u32::MAX..u32::MAX, after: u32::MAX..u32::MAX, }; - /// Inverts a change so that `before` + /// Inverts a change so that `before` becomes `after` and `after` becomes `before` pub fn invert(&self) -> Hunk { Hunk { before: self.after.clone(), diff --git a/helix-vcs/src/diff/line_cache.rs b/helix-vcs/src/diff/line_cache.rs index 33c8c45eccd1..c3ee5daa3e79 100644 --- a/helix-vcs/src/diff/line_cache.rs +++ b/helix-vcs/src/diff/line_cache.rs @@ -55,8 +55,8 @@ impl InternedRopeLines { } } - /// Updates the `doc` without reintering the `diff_base`, this funciton - /// is therefore significantly faster then `update_diff_base` when only the document changes. + /// Updates the `doc` without reinterning the `diff_base`, this function + /// is therefore significantly faster than `update_diff_base` when only the document changes. pub fn update_doc(&mut self, doc: Rope) { // Safety: we clear any tokens that were added after // the interning of `self.diff_base` finished so @@ -75,7 +75,7 @@ impl InternedRopeLines { } fn update_diff_base_impl(&mut self) { - // Safety: This transmute is save because it only transmutes a lifetime, which has no effect. + // Safety: This transmute is safe because it only transmutes a lifetime, which has no effect. // The backing storage for the RopeSlices referred to by the lifetime is stored in `self.diff_base`. // Therefore as long as `self.diff_base` is not dropped/replaced this memory remains valid. // `self.diff_base` is only changed in `self.update_diff_base`, which clears the interner. diff --git a/helix-vcs/src/diff/worker.rs b/helix-vcs/src/diff/worker.rs index b83bf471d8f2..bcbd6ac9e2a0 100644 --- a/helix-vcs/src/diff/worker.rs +++ b/helix-vcs/src/diff/worker.rs @@ -121,7 +121,7 @@ impl<'a> EventAccumulator<'a> { *dst = Some(event.text); - // always prefer the most synchronus requested render mode + // always prefer the most synchronous requested render mode if event.render_strategy > self.render_stratagey { if self.render_lock.is_none() { self.timeout = Instant::now() + Duration::from_millis(SYNC_DIFF_TIMEOUT); @@ -154,7 +154,7 @@ impl<'a> EventAccumulator<'a> { } // setup task to trigger the rendering - // with the choosen render stragey + // with the chosen render stragey match self.render_stratagey { RenderStrategy::Async => { tokio::spawn(async move { @@ -166,8 +166,8 @@ impl<'a> EventAccumulator<'a> { let timeout = self.timeout; tokio::spawn(async move { let res = { - // Aquire a lock on the redraw handle. - // The lock will block the rendering from occuring while held. + // Acquire a lock on the redraw handle. + // The lock will block the rendering from occurring while held. // The rendering waits for the diff if it doesn't time out let _render_guard = redraw_handle.1.read(); timeout_at(timeout, diff_finished_notify.notified()).await diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index dd4ea76a8f88..37aca84410ce 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -154,7 +154,7 @@ pub struct Config { )] pub idle_timeout: Duration, /// Time in milliseconds since last keypress before a redraws trigger. - /// Used for redrawing asynchronsouly computed UI compoenents, set to 0 for instant. + /// Used for redrawing asynchronsouly computed UI components, set to 0 for instant. /// Defaults to 100ms. #[serde( serialize_with = "serialize_duration_millis", diff --git a/helix-view/src/gutter.rs b/helix-view/src/gutter.rs index c1de8054e54d..377518fb5379 100644 --- a/helix-view/src/gutter.rs +++ b/helix-view/src/gutter.rs @@ -101,10 +101,10 @@ pub fn diff<'doc>( let mut hunk = hunks.nth_hunk(hunk_i); Box::new(move |line: usize, _selected: bool, out: &mut String| { // truncating the line is fine here because we don't compute diffs - // for files with more lines then i32::MAX anyways + // for files with more lines than i32::MAX anyways // we need to special case removals here // these technically do not have a range of lines to highlight (`hunk.after.start == hunk.after.end`). - // However we still want to display these hunks correctly we must not yet scipe to the next hunk here + // However we still want to display these hunks correctly we must not yet skip to the next hunk here while hunk.after.end < line as u32 || !hunk.is_pure_removal() && line as u32 == hunk.after.end { From bc119ba1b25a3a7bc6d2a81c553a1184612c651e Mon Sep 17 00:00:00 2001 From: Pascal Kuthe <pascal.kuthe@semimod.de> Date: Mon, 28 Nov 2022 19:01:11 +0100 Subject: [PATCH 4/5] address review comments and ensure lock is always aquired --- helix-term/src/application.rs | 2 +- helix-vcs/src/diff.rs | 64 +++++++++++++--------- helix-vcs/src/diff/worker.rs | 91 +++++++++++++++++-------------- helix-vcs/src/diff/worker/test.rs | 4 +- helix-view/src/editor.rs | 8 +-- 5 files changed, 95 insertions(+), 74 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index b62d8713b082..dc12ba3cddf5 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -284,7 +284,7 @@ impl Application { scroll: None, }; - // Aquire mutable access to the redraw_handle lock + // Acquire mutable access to the redraw_handle lock // to ensure that there are no tasks running that want to block rendering drop(cx.editor.redraw_handle.1.write().await); cx.editor.needs_redraw = false; diff --git a/helix-vcs/src/diff.rs b/helix-vcs/src/diff.rs index 240b30f646e1..b1acd1f29937 100644 --- a/helix-vcs/src/diff.rs +++ b/helix-vcs/src/diff.rs @@ -5,34 +5,33 @@ use helix_core::Rope; use imara_diff::Algorithm; use parking_lot::{Mutex, MutexGuard}; use tokio::sync::mpsc::{unbounded_channel, UnboundedSender}; -use tokio::sync::{Notify, RwLock}; +use tokio::sync::{Notify, OwnedRwLockReadGuard, RwLock}; use tokio::task::JoinHandle; +use tokio::time::Instant; use crate::diff::worker::DiffWorker; mod line_cache; mod worker; -type RedrawHandle = Arc<(Notify, RwLock<()>)>; +type RedrawHandle = (Arc<Notify>, Arc<RwLock<()>>); -// The order of enum variants is used by the PartialOrd -// derive macro, DO NOT REORDER -#[derive(PartialEq, PartialOrd)] -enum RenderStrategy { - Async, - SyncWithTimeout, - Sync, +/// A rendering lock passed to the differ the prevents redraws from occurring +struct RenderLock { + pub lock: OwnedRwLockReadGuard<()>, + pub timeout: Option<Instant>, } struct Event { text: Rope, is_base: bool, - render_strategy: RenderStrategy, + render_lock: Option<RenderLock>, } #[derive(Clone, Debug)] pub struct DiffHandle { channel: UnboundedSender<Event>, + render_lock: Arc<RwLock<()>>, hunks: Arc<Mutex<Vec<Hunk>>>, inverted: bool, } @@ -53,14 +52,15 @@ impl DiffHandle { channel: receiver, hunks: hunks.clone(), new_hunks: Vec::default(), - redraw_handle, - difff_finished_notify: Arc::default(), + redraw_notify: redraw_handle.0, + diff_finished_notify: Arc::default(), }; let handle = tokio::spawn(worker.run(diff_base, doc)); let differ = DiffHandle { channel: sender, hunks, inverted: false, + render_lock: redraw_handle.1, }; (differ, handle) } @@ -76,36 +76,48 @@ impl DiffHandle { } } + /// Updates the document associated with this redraw handle + /// This function is only intended to be called from within the rendering loop + /// if called from elsewhere it may fail to acquire the render lock and panic pub fn update_document(&self, doc: Rope, block: bool) -> bool { - let mode = if block { - RenderStrategy::Sync + // unwrap is ok here because the rendering lock is + // only exclusively locked during redraw. + // This function is only intended to be called + // from the core rendering loop where no redraw can happen in parallel + let lock = self.render_lock.clone().try_read_owned().unwrap(); + let timeout = if block { + None } else { - RenderStrategy::SyncWithTimeout + Some(Instant::now() + tokio::time::Duration::from_millis(SYNC_DIFF_TIMEOUT)) }; - self.update_document_impl(doc, self.inverted, mode) + self.update_document_impl(doc, self.inverted, Some(RenderLock { lock, timeout })) } pub fn update_diff_base(&self, diff_base: Rope) -> bool { - self.update_document_impl(diff_base, !self.inverted, RenderStrategy::Async) + self.update_document_impl(diff_base, !self.inverted, None) } - fn update_document_impl(&self, text: Rope, is_base: bool, mode: RenderStrategy) -> bool { + fn update_document_impl( + &self, + text: Rope, + is_base: bool, + render_lock: Option<RenderLock>, + ) -> bool { let event = Event { text, is_base, - render_strategy: mode, + render_lock, }; self.channel.send(event).is_ok() } } -// TODO configuration /// synchronous debounce value should be low /// so we can update synchronously most of the time const DIFF_DEBOUNCE_TIME_SYNC: u64 = 1; /// maximum time that rendering should be blocked until the diff finishes -const SYNC_DIFF_TIMEOUT: u64 = 50; -const DIFF_DEBOUNCE_TIME_ASYNC: u64 = 100; +const SYNC_DIFF_TIMEOUT: u64 = 12; +const DIFF_DEBOUNCE_TIME_ASYNC: u64 = 96; const ALGORITHM: Algorithm = Algorithm::Histogram; const MAX_DIFF_LINES: usize = 64 * u16::MAX as usize; // cap average line length to 128 for files with MAX_DIFF_LINES @@ -128,15 +140,15 @@ pub struct Hunk { impl Hunk { /// Can be used instead of `Option::None` for better performance - /// because lines larger than `i32::MAX` are not supported by imara-diff anways. - /// Has some nice properties where it usually is not necessary to check for `None` seperatly: - /// Empty ranges fail contains checks and also fails smaller than checks. + /// because lines larger then `i32::MAX` are not supported by `imara-diff` anyways. + /// Has some nice properties where it usually is not necessary to check for `None` separately: + /// Empty ranges fail contains checks and also fails smaller then checks. pub const NONE: Hunk = Hunk { before: u32::MAX..u32::MAX, after: u32::MAX..u32::MAX, }; - /// Inverts a change so that `before` becomes `after` and `after` becomes `before` + /// Inverts a change so that `before` pub fn invert(&self) -> Hunk { Hunk { before: self.after.clone(), diff --git a/helix-vcs/src/diff/worker.rs b/helix-vcs/src/diff/worker.rs index bcbd6ac9e2a0..b8659c9b3bc2 100644 --- a/helix-vcs/src/diff/worker.rs +++ b/helix-vcs/src/diff/worker.rs @@ -6,12 +6,11 @@ use helix_core::{Rope, RopeSlice}; use imara_diff::intern::InternedInput; use parking_lot::Mutex; use tokio::sync::mpsc::UnboundedReceiver; -use tokio::sync::{Notify, RwLockReadGuard}; -use tokio::time::{timeout, timeout_at, Duration, Instant}; +use tokio::sync::Notify; +use tokio::time::{timeout, timeout_at, Duration}; use crate::diff::{ - Event, RedrawHandle, RenderStrategy, ALGORITHM, DIFF_DEBOUNCE_TIME_ASYNC, - DIFF_DEBOUNCE_TIME_SYNC, SYNC_DIFF_TIMEOUT, + Event, RenderLock, ALGORITHM, DIFF_DEBOUNCE_TIME_ASYNC, DIFF_DEBOUNCE_TIME_SYNC, }; use super::line_cache::InternedRopeLines; @@ -24,19 +23,19 @@ pub(super) struct DiffWorker { pub channel: UnboundedReceiver<Event>, pub hunks: Arc<Mutex<Vec<Hunk>>>, pub new_hunks: Vec<Hunk>, - pub redraw_handle: RedrawHandle, - pub difff_finished_notify: Arc<Notify>, + pub redraw_notify: Arc<Notify>, + pub diff_finished_notify: Arc<Notify>, } impl DiffWorker { async fn accumulate_events(&mut self, event: Event) -> (Option<Rope>, Option<Rope>) { - let mut accumulator = EventAccumulator::new(&self.redraw_handle); + let mut accumulator = EventAccumulator::new(); accumulator.handle_event(event).await; accumulator .accumulate_debounced_events( &mut self.channel, - self.redraw_handle.clone(), - self.difff_finished_notify.clone(), + self.redraw_notify.clone(), + self.diff_finished_notify.clone(), ) .await; (accumulator.doc, accumulator.diff_base) @@ -80,7 +79,7 @@ impl DiffWorker { /// To improve performance this function tries to reuse the allocation of the old diff previously stored in `self.line_diffs` fn apply_hunks(&mut self) { swap(&mut *self.hunks.lock(), &mut self.new_hunks); - self.difff_finished_notify.notify_waiters(); + self.diff_finished_notify.notify_waiters(); self.new_hunks.clear(); } @@ -91,24 +90,18 @@ impl DiffWorker { } } -struct EventAccumulator<'a> { +struct EventAccumulator { diff_base: Option<Rope>, doc: Option<Rope>, - render_stratagey: RenderStrategy, - redraw_handle: &'a RedrawHandle, - render_lock: Option<RwLockReadGuard<'a, ()>>, - timeout: Instant, + render_lock: Option<RenderLock>, } -impl<'a> EventAccumulator<'a> { - fn new(redraw_handle: &'a RedrawHandle) -> EventAccumulator<'a> { +impl<'a> EventAccumulator { + fn new() -> EventAccumulator { EventAccumulator { diff_base: None, doc: None, - render_stratagey: RenderStrategy::Async, render_lock: None, - redraw_handle, - timeout: Instant::now(), } } @@ -122,25 +115,33 @@ impl<'a> EventAccumulator<'a> { *dst = Some(event.text); // always prefer the most synchronous requested render mode - if event.render_strategy > self.render_stratagey { - if self.render_lock.is_none() { - self.timeout = Instant::now() + Duration::from_millis(SYNC_DIFF_TIMEOUT); - self.render_lock = Some(self.redraw_handle.1.read().await); + if let Some(render_lock) = event.render_lock { + match &mut self.render_lock { + Some(RenderLock { timeout, .. }) => { + // A timeout of `None` means that the render should + // always wait for the diff to complete (so no timeout) + // remove the existing timeout, otherwise keep the previous timeout + // because it will be shorter then the current timeout + if render_lock.timeout.is_none() { + timeout.take(); + } + } + None => self.render_lock = Some(render_lock), } - self.render_stratagey = event.render_strategy } } async fn accumulate_debounced_events( &mut self, channel: &mut UnboundedReceiver<Event>, - redraw_handle: RedrawHandle, + redraw_notify: Arc<Notify>, diff_finished_notify: Arc<Notify>, ) { let async_debounce = Duration::from_millis(DIFF_DEBOUNCE_TIME_ASYNC); let sync_debounce = Duration::from_millis(DIFF_DEBOUNCE_TIME_SYNC); loop { - let debounce = if self.render_stratagey == RenderStrategy::Async { + // if we are not blocking rendering use a much longer timeout + let debounce = if self.render_lock.is_none() { async_debounce } else { sync_debounce @@ -154,24 +155,30 @@ impl<'a> EventAccumulator<'a> { } // setup task to trigger the rendering - // with the chosen render stragey - match self.render_stratagey { - RenderStrategy::Async => { + match self.render_lock.take() { + // diff is performed outside of the rendering loop + // request a redraw after the diff is done + None => { tokio::spawn(async move { diff_finished_notify.notified().await; - redraw_handle.0.notify_one(); + redraw_notify.notify_one(); }); } - RenderStrategy::SyncWithTimeout => { - let timeout = self.timeout; + // diff is performed inside the rendering loop + // block redraw until the diff is done or the timeout is expired + Some(RenderLock { + lock, + timeout: Some(timeout), + }) => { tokio::spawn(async move { let res = { // Acquire a lock on the redraw handle. // The lock will block the rendering from occurring while held. // The rendering waits for the diff if it doesn't time out - let _render_guard = redraw_handle.1.read(); timeout_at(timeout, diff_finished_notify.notified()).await }; + // we either reached the timeout or the diff is finished, release the render lock + drop(lock); if res.is_ok() { // Diff finished in time we are done. return; @@ -180,15 +187,19 @@ impl<'a> EventAccumulator<'a> { // and wait until the diff occurs to trigger an async redraw log::warn!("Diff computation timed out, update of diffs might appear delayed"); diff_finished_notify.notified().await; - redraw_handle.0.notify_one(); + redraw_notify.notify_one(); }); } - RenderStrategy::Sync => { + // a blocking diff is performed inside the rendering loop + // block redraw until the diff is done + Some(RenderLock { + lock, + timeout: None, + }) => { tokio::spawn(async move { - // Aquire a lock on the redraw handle. - // The lock will block the rendering from occuring while held. - let _render_guard = redraw_handle.1.read(); - diff_finished_notify.notified().await + diff_finished_notify.notified().await; + // diff is done release the lock + drop(lock) }); } }; diff --git a/helix-vcs/src/diff/worker/test.rs b/helix-vcs/src/diff/worker/test.rs index 88dea9c7de5f..1444242651c8 100644 --- a/helix-vcs/src/diff/worker/test.rs +++ b/helix-vcs/src/diff/worker/test.rs @@ -1,5 +1,3 @@ -use std::sync::Arc; - use helix_core::Rope; use tokio::task::JoinHandle; @@ -10,7 +8,7 @@ impl DiffHandle { DiffHandle::new_with_handle( Rope::from_str(diff_base), Rope::from_str(doc), - Arc::default(), + Default::default(), ) } async fn into_diff(self, handle: JoinHandle<()>) -> Vec<Hunk> { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 37aca84410ce..7b8e448aff5b 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -154,7 +154,7 @@ pub struct Config { )] pub idle_timeout: Duration, /// Time in milliseconds since last keypress before a redraws trigger. - /// Used for redrawing asynchronsouly computed UI components, set to 0 for instant. + /// Used for redrawing asynchronously computed UI components, set to 0 for instant. /// Defaults to 100ms. #[serde( serialize_with = "serialize_duration_millis", @@ -733,11 +733,11 @@ pub struct Editor { /// Allows asynchronous tasks to control the rendering /// The `Notify` allows asynchronous tasks to request the editor to perform a redraw /// The `RwLock` blocks the editor from performing the render until an exclusive lock can be aquired - pub redraw_handle: Arc<(Notify, RwLock<()>)>, + pub redraw_handle: RedrawHandle, pub needs_redraw: bool, } -pub type RedrawHandle = Arc<(Notify, RwLock<()>)>; +pub type RedrawHandle = (Arc<Notify>, Arc<RwLock<()>>); #[derive(Debug)] pub enum EditorEvent { @@ -830,7 +830,7 @@ impl Editor { auto_pairs, exit_code: 0, config_events: unbounded_channel(), - redraw_handle: Arc::default(), + redraw_handle: Default::default(), needs_redraw: false, } } From 8bbf8783df1d7312a4bfec174c2195dbb7942ef4 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe <pascal.kuthe@semimod.de> Date: Wed, 30 Nov 2022 10:56:12 +0100 Subject: [PATCH 5/5] remove configuration for redraw timeout --- helix-view/src/editor.rs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 7b8e448aff5b..d3b85c511155 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -153,14 +153,6 @@ pub struct Config { deserialize_with = "deserialize_duration_millis" )] pub idle_timeout: Duration, - /// Time in milliseconds since last keypress before a redraws trigger. - /// Used for redrawing asynchronously computed UI components, set to 0 for instant. - /// Defaults to 100ms. - #[serde( - serialize_with = "serialize_duration_millis", - deserialize_with = "deserialize_duration_millis" - )] - pub redraw_timeout: Duration, pub completion_trigger_len: u8, /// Whether to display infoboxes. Defaults to true. pub auto_info: bool, @@ -638,7 +630,6 @@ impl Default for Config { bufferline: BufferLine::default(), indent_guides: IndentGuidesConfig::default(), color_modes: false, - redraw_timeout: Duration::from_millis(200), } } } @@ -866,11 +857,6 @@ impl Editor { .reset(Instant::now() + config.idle_timeout); } - fn redraw_deadline(&self) -> Instant { - let config = self.config(); - Instant::now() + config.redraw_timeout - } - pub fn clear_status(&mut self) { self.status_msg = None; } @@ -1399,7 +1385,7 @@ impl Editor { _ = self.redraw_handle.0.notified() => { if !self.needs_redraw{ self.needs_redraw = true; - let timeout = self.redraw_deadline(); + let timeout = Instant::now() + Duration::from_millis(96); if timeout < self.idle_timer.deadline(){ self.idle_timer.as_mut().reset(timeout) }