Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/majutsu.org
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ Press ~d~ to open the Diff transient.

*** Diff Buffer
The diff buffer is highly interactive:
- ~RET~ :: Visit the file in your workspace at the corresponding line.
- ~RET~ :: Visit the appropriate version of the file at point. For working copy diffs, visits the actual file. For committed changes, visits the blob.
- ~C-j~ / ~C-<return>~ :: Visit the workspace file, regardless of diff type. Note: when Evil mode is active, ~C-j~ may be overridden by Evil's section navigation; use ~C-<return>~ instead, which is unaffected.
- ~+~ / ~-~ :: Increase or decrease the amount of context shown.
- ~t~ :: Toggle word-level refinement.

Expand Down
28 changes: 20 additions & 8 deletions majutsu-base.el
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,6 @@ KIND (a symbol such as `log', `diff' or `message') via
(or (get-buffer-window buffer t)
(selected-window))))

(defun majutsu--normalize-id-value (value)
"Normalize VALUE (string/symbol/number) into a plain string without
text properties."
(cond
((stringp value) (substring-no-properties value))
((and value (not (stringp value))) (format "%s" value))
(t nil)))

(defun majutsu--buffer-root (&optional buffer)
"Return the cached root for BUFFER (default `current-buffer').

Expand Down Expand Up @@ -263,6 +255,26 @@ of the selected frame."
(unless (derived-mode-p mode)
(user-error "Command is only valid in %s buffers" mode)))

;;; Change at Point

(defvar majutsu-buffer-blob-revision)

(defun majutsu-revision-at-point ()
"Return the change-id at point.
This checks multiple sources in order:
1. Section value (jj-commit section)
2. Blob buffer revision
3. Diff buffer revision"
(or (magit-section-value-if 'jj-commit)
(and (bound-and-true-p majutsu-buffer-blob-revision)
majutsu-buffer-blob-revision)
(and (derived-mode-p 'majutsu-diff-mode)
(bound-and-true-p majutsu-buffer-diff-range)
(let ((range majutsu-buffer-diff-range))
(or (and (equal (car range) "-r") (cadr range))
(when-let* ((arg (seq-find (lambda (item) (string-prefix-p "--revisions=" item)) range)))
(substring arg (length "--revisions="))))))))

;;; _
(provide 'majutsu-base)
;;; majutsu-base.el ends here
193 changes: 144 additions & 49 deletions majutsu-diff.el
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
(require 'majutsu-config)
(require 'majutsu-selection)
(require 'majutsu-section)
(require 'majutsu-file)
(require 'magit-diff) ; for faces/font-lock keywords
(require 'diff-mode)
(require 'smerge-mode)
Expand Down Expand Up @@ -88,8 +89,6 @@ otherwise fall back to the current buffer's `tab-width'."
"Face for the (binary) label in diffstat entries."
:group 'majutsu)

;;;

(defvar majutsu-diff--tab-width-cache nil
"Alist mapping file names to cached tab widths.")

Expand Down Expand Up @@ -303,7 +302,7 @@ The list is in the same order as the diff headers appear."
(goto-char (point-min))
(let (files)
(while (re-search-forward "^diff --git a/\\(.*\\) b/\\(.*\\)$" nil t)
(push (match-string 2) files))
(push (match-string-no-properties 2) files))
(nreverse files))))

(defun majutsu-jump-to-diffstat-or-diff (&optional expand)
Expand Down Expand Up @@ -418,7 +417,7 @@ first \"diff --git\" header."
(magit-insert-heading
(propertize heading 'font-lock-face 'magit-diff-file-heading))
(while (looking-at majutsu-diff-statline-re)
(let* ((file (match-string 1))
(let* ((file (match-string-no-properties 1))
(sep (match-string 2))
(cnt (match-string 3))
(add (match-string 4))
Expand Down Expand Up @@ -472,8 +471,8 @@ Assumes point is at the start of the diff output."
(defun majutsu-diff-wash-file ()
"Parse a single file section at point and wrap it in Magit sections."
(when (looking-at "^diff --git a/\\(.*\\) b/\\(.*\\)$")
(let* ((file-a (match-string 1))
(file-b (match-string 2))
(let* ((file-a (match-string-no-properties 1))
(file-b (match-string-no-properties 2))
(file (or file-b file-a))
(headers nil)
(diff-header (buffer-substring-no-properties
Expand Down Expand Up @@ -698,31 +697,146 @@ works with the simplified jj diff we render here."
(when-let* ((file (majutsu-file-at-point)))
(find-file (expand-file-name file default-directory))))

(defun majutsu-diff--range-value (range prefix)
"Return the value in RANGE for argument starting with PREFIX."
(when range
(when-let* ((arg (seq-find (lambda (item) (string-prefix-p prefix item)) range)))
(substring arg (length prefix)))))

(defun majutsu-diff--on-removed-line-p ()
"Return non-nil if point is on a removed diff line."
(eq (char-after (line-beginning-position)) ?-))

(defun majutsu-diff--default-revset ()
"Return the revset implied by the current diff buffer."
(let* ((range majutsu-buffer-diff-range)
(removed (majutsu-diff--on-removed-line-p))
(from (majutsu-diff--range-value range "--from="))
(to (majutsu-diff--range-value range "--to="))
(revisions (majutsu-diff--range-value range "--revisions=")))
(cond
((and range (equal (car range) "-r") (cadr range)) (cadr range))
(revisions revisions)
(from (if (and removed from) from (or to from)))
(t "@"))))
Comment on lines +710 to +721
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

jj diff default behavior working copy parent revision

💡 Result:

In Jujutsu (jj):

  • jj diff (no args) defaults to jj diff -r @, i.e. it shows the diff for the working-copy revision @ compared to its parent revision @-. If @ is a merge, jj effectively merges the parents and diffs that merge-result against @. [1][2]
  • jj diff -r <REV> means “diff <REV> vs its parent <REV>-”. [1][2]
  • jj diff --from A (with no --to) means “diff A → @”, because an omitted side defaults to the working-copy commit. Likewise, jj diff --to B means “diff @ → B”. [1][2]
  • By default jj snapshots the working copy at the start of each command, so jj diff reflects your current working directory state unless you pass --ignore-working-copy. [2]

Sources: Jujutsu CLI reference / jj diff docs [1], jj-diff(1) man page [2].


🌐 Web query:

jj removed lines diff revision mapping @-``

💡 Result:

In Jujutsu (jj) revsets, @ means “the working-copy change”, and @- means the parent of the working copy (the change you’re based on). (jj-vcs.github.io)

How to interpret “removed lines” in a jj diff

  • jj diff (by default) is effectively comparing @- → @ (parent → working copy). (jj-vcs.github.io)
  • In a Git-style diff (--git), lines starting with - are lines that exist in the from revision (e.g., @-) but not in the to revision (e.g., @). The hunk header @@ -old_start,old_len +new_start,new_len @@ is the line-range mapping for old vs new.

Useful commands:

jj diff --git                 # shows changes from `@-` to @ in familiar unified-diff form
jj diff --from `@-` --to @ --git

(jj-vcs.github.io)

If you want the revision associated with removed lines

Once a line is deleted, it won’t appear in annotate of @. Instead, annotate the file in @- (before the deletion):

jj file annotate -r `@-` path/to/file

This shows which change introduced each line in that file version. (docs.jj-vcs.dev)

Citations:


🏁 Script executed:

#!/bin/bash
# Check the actual code context around lines 761-772 to verify
# coding style compliance and no other issues
cd /tmp && find . -name "majutsu-diff.el" 2>/dev/null | head -1

Repository: 0WD0/majutsu

Length of output: 38


🏁 Script executed:

#!/bin/bash
# Search for majutsu-diff.el in the repo
fd "majutsu-diff.el" -type f

Repository: 0WD0/majutsu

Length of output: 226


🏁 Script executed:

#!/bin/bash
# Find majutsu-diff.el in the repository
fd "majutsu-diff.el"

Repository: 0WD0/majutsu

Length of output: 71


🏁 Script executed:

#!/bin/bash
# Read the actual code at lines 761-772
sed -n '761,772p' majutsu-diff.el

Repository: 0WD0/majutsu

Length of output: 612


🏁 Script executed:

#!/bin/bash
# Get more context around those lines to see the function and verify the logic
sed -n '755,780p' majutsu-diff.el

Repository: 0WD0/majutsu

Length of output: 1101


Add fallback to @- for removed lines in default diffs.

When jj diff runs with no explicit range, it compares @- (parent) to @ (working copy). Removed lines exist in @- but not in @. The current code falls through to (t "@"), which tries to open the removed line in the working copy where it doesn't exist.

Suggested fix
 (defun majutsu-diff--default-revset ()
   "Return the revset implied by the current diff buffer."
   (let* ((range majutsu-buffer-diff-range)
          (removed (majutsu-diff--on-removed-line-p))
          (from (majutsu-diff--range-value range "--from="))
          (to (majutsu-diff--range-value range "--to="))
          (revisions (majutsu-diff--range-value range "--revisions=")))
     (cond
      ((and range (equal (car range) "-r") (cadr range)) (cadr range))
      (revisions revisions)
      (from (if (and removed from) from (or to from)))
+     ((and removed (null range)) "@-")
      (t "@"))))
🤖 Prompt for AI Agents
In `@majutsu-diff.el` around lines 761 - 772, majutsu-diff--default-revset should
return the parent revset "@-" when the current diff buffer indicates we're on a
removed line; update the logic in majutsu-diff--default-revset to detect removed
lines via majutsu-diff--on-removed-line-p and, when removed and no explicit
--revisions/--from/--to provide a usable revset, return "@-" instead of falling
through to the default "@". Ensure the existing checks for (revisions revisions)
and (from ...) remain, but change the final fallback branch that currently
returns "@" to return "@-" when (majutsu-diff--on-removed-line-p) is true,
otherwise keep "@".


(defun majutsu-diff--hunk-line (section goto-from)
"Return the line number in SECTION for GOTO-FROM side."
(with-slots (content from-range to-range) section
(let ((start (car (if goto-from from-range to-range))))
(when start
(let ((line start)
(target (point)))
(save-excursion
(goto-char content)
(while (< (point) target)
(let ((ch (char-after (line-beginning-position))))
(cond
((eq ch ?+) (unless goto-from (setq line (1+ line))))
((eq ch ?-) (when goto-from (setq line (1+ line))))
(t (setq line (1+ line)))))
(forward-line 1)))
line)))))

(defun majutsu-diff--hunk-column (section goto-from)
"Return the column for SECTION based on GOTO-FROM side."
(let ((bol (line-beginning-position)))
(if (or (< (point) (oref section content))
(and (not goto-from) (eq (char-after bol) ?-)))
0
(let ((col (current-column)))
(if (memq (char-after bol) '(?+ ?-))
(max 0 (1- col))
col)))))
Comment on lines +741 to +750
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Column is off-by-one on context lines.

In unified diff format, context lines are prefixed with a space character. The current code only adjusts for ?+ and ?- markers but not the space prefix on context lines, so the column will be 1 too large when visiting from a context line.

Suggested fix
       (let ((col (current-column)))
-        (if (memq (char-after bol) '(?+ ?-))
+        (if (memq (char-after bol) '(?+ ?- ?\s))
             (max 0 (1- col))
           col)))))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(defun majutsu-diff--hunk-column (section goto-from)
"Return the column for SECTION based on GOTO-FROM side."
(let ((bol (line-beginning-position)))
(if (or (< (point) (oref section content))
(and (not goto-from) (eq (char-after bol) ?-)))
0
(let ((col (current-column)))
(if (memq (char-after bol) '(?+ ?-))
(max 0 (1- col))
col)))))
(defun majutsu-diff--hunk-column (section goto-from)
"Return the column for SECTION based on GOTO-FROM side."
(let ((bol (line-beginning-position)))
(if (or (< (point) (oref section content))
(and (not goto-from) (eq (char-after bol) ?-)))
0
(let ((col (current-column)))
(if (memq (char-after bol) '(?+ ?- ?\s))
(max 0 (1- col))
col)))))
🤖 Prompt for AI Agents
In `@majutsu-diff.el` around lines 741 - 750, The column computation in
majutsu-diff--hunk-column fails to account for unified-diff context lines (which
are prefixed with a space), causing an off-by-one; update the branch that checks
(memq (char-after bol) '(?+ ?-)) in majutsu-diff--hunk-column to also treat a
space prefix (?\s or ?\ ) as a "marker" and apply the same (max 0 (1- col))
adjustment when char-after bol is a space so context lines yield the correct
column when visiting from a context line.


(defun majutsu-diff--goto-line-col (buffer line col)
"Move point in BUFFER to LINE and COL."
(with-current-buffer buffer
(widen)
(goto-char (point-min))
(forward-line (max 0 (1- line)))
(move-to-column col)))

(defun majutsu-diff--visit-workspace-p ()
"Return non-nil if the current diff should visit the workspace file.
This is true when diffing the working copy (@) on the new/right side."
(let* ((range majutsu-buffer-diff-range)
(to (majutsu-diff--range-value range "--to="))
(revisions (majutsu-diff--range-value range "--revisions=")))
(cond
;; Explicit --to=@ means we're looking at working copy changes
((equal to "@") t)
;; No range specified defaults to -r @ (working copy)
((null range) t)
;; Single revision diff (-r @) shows working copy
((and revisions (equal revisions "@")) t)
;; Otherwise we're looking at committed changes
(t nil))))

;;;###autoload
(defun majutsu-diff-visit-file ()
"Visit the file at point.
(defun majutsu-diff-visit-file (&optional force-workspace)
"From a diff, visit the appropriate version of the file at point.

When point is on a hunk section, jump to the corresponding line in the
file."
(interactive)
(let ((section (magit-current-section)))
(cond
((and section (magit-section-match 'jj-hunk section))
(majutsu-goto-diff-line))
((majutsu-file-at-point)
(majutsu-visit-file))
(t
(user-error "No file at point")))))
If point is on an added or context line, visit the new/right side.
If point is on a removed line, visit the old/left side.

For diffs of the working copy (@), this visits the actual file in
the workspace. For diffs of committed changes, this visits the
blob from the appropriate revision.

With prefix argument FORCE-WORKSPACE, always visit the workspace file
regardless of what the diff is about."
(interactive "P")
(let* ((section (magit-current-section))
(file (majutsu-file-at-point)))
(unless file
(user-error "No file at point"))
(let* ((goto-from (and section (magit-section-match 'jj-hunk section)
(majutsu-diff--on-removed-line-p)))
(goto-workspace (or force-workspace
(and (majutsu-diff--visit-workspace-p)
(not goto-from))))
(line (and section (magit-section-match 'jj-hunk section)
(majutsu-diff--hunk-line section goto-from)))
(col (and section (magit-section-match 'jj-hunk section)
(majutsu-diff--hunk-column section goto-from))))
(if goto-workspace
;; Visit workspace file
(let ((full-path (expand-file-name file default-directory)))
(if (file-exists-p full-path)
(progn
(find-file full-path)
(when (and line col)
(goto-char (point-min))
(forward-line (1- line))
(move-to-column col)))
(user-error "File does not exist in workspace: %s" file)))
;; Visit blob
(let* ((revset (majutsu-diff--default-revset))
(buf (majutsu-find-file revset file)))
(when (and buf line col)
(majutsu-diff--goto-line-col buf line col)))))))

;;; Section Keymaps

(defvar-keymap majutsu-diff-section-map
:doc "Keymap for diff sections."
"<remap> <majutsu-visit-thing>" #'majutsu-diff-visit-file)
"<remap> <majutsu-visit-thing>" #'majutsu-diff-visit-file
"C-j" #'majutsu-diff-visit-workspace-file
"C-<return>" #'majutsu-diff-visit-workspace-file)

;;;###autoload
(defun majutsu-diff-visit-workspace-file ()
"From a diff, visit the workspace version of the file at point.
Always visits the actual file in the working tree, regardless of
what the diff is about."
(interactive)
(majutsu-diff-visit-file t))

(defvar-keymap majutsu-file-section-map
:doc "Keymap for `jj-file' sections."
:parent majutsu-diff-section-map)
:parent majutsu-diff-section-map
"v" #'majutsu-find-file-at-point)

(defvar-keymap majutsu-hunk-section-map
:doc "Keymap for `jj-hunk' sections."
Expand Down Expand Up @@ -858,21 +972,6 @@ file."

;;; Diff Commands

(defun majutsu-diff-clear-selections ()
"Clear all diff selections."
(interactive)
(majutsu-selection-clear 'from)
(majutsu-selection-clear 'to)
(when (consp transient--suffixes)
(dolist (obj transient--suffixes)
(when (and (cl-typep obj 'majutsu-diff-range-option)
(memq (oref obj selection-key) '(revisions from to)))
(transient-infix-set obj nil))))
(when transient--prefix
(transient--redisplay))
(when (called-interactively-p 'interactive)
(message "Cleared diff selections")))

(defun majutsu-diff-less-context (&optional count)
"Decrease the context for diff hunks by COUNT lines."
(interactive "p")
Expand Down Expand Up @@ -942,17 +1041,13 @@ With prefix STYLE, cycle between `all' and `t'."
"Refresh the current diff buffer."
(interactive)
(when majutsu-buffer-diff-args
(let* (;; Avoid ANSI; let our painting run lazily.
(majutsu-jj-global-arguments
(cons "--color=never"
(seq-remove (lambda (arg)
(string-prefix-p "--color" arg))
majutsu-jj-global-arguments)))
(majutsu-process-apply-ansi-colors nil))
(magit-insert-section (diffbuf)
(magit-run-section-hook 'majutsu-diff-sections-hook))
(when (eq majutsu-diff-refine-hunk 'all)
(majutsu-diff--update-hunk-refinement)))))
;; Avoid ANSI; let our painting run lazily.
(majutsu--with-no-color
(let ((majutsu-process-apply-ansi-colors nil))
(magit-insert-section (diffbuf)
(magit-run-section-hook 'majutsu-diff-sections-hook))
(when (eq majutsu-diff-refine-hunk 'all)
(majutsu-diff--update-hunk-refinement))))))

;;;###autoload
(defun majutsu-diff-dwim (&optional args range filesets)
Expand All @@ -962,7 +1057,7 @@ With prefix STYLE, cycle between `all' and `t'."
(`(commit . ,rev) rev)
(_ "@")))
(range (or range
(list (concat "--revisions=" (majutsu--normalize-id-value rev))))))
(list (concat "--revisions=" (substring-no-properties rev))))))
(majutsu-diff-setup-buffer args range filesets)))

;;;###autoload
Expand All @@ -972,7 +1067,7 @@ With prefix STYLE, cycle between `all' and `t'."
REVSET is passed to jj diff using `--revisions='."
(interactive (cons (majutsu-read-revset "Diff revset")
(majutsu-diff-arguments)))
(let ((range (list (concat "--revisions=" (majutsu--normalize-id-value revset)))))
(let ((range (list (concat "--revisions=" (substring-no-properties revset)))))
(majutsu-diff-setup-buffer args range filesets)))

;; TODO: implement more DWIM cases
Expand Down
11 changes: 6 additions & 5 deletions majutsu-edit.el
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@

With prefix ARG, pass --ignore-immutable."
(interactive "P")
(when-let* ((revset (magit-section-value-if 'jj-commit))
(args (append (list "edit" revset)
(when arg (list "--ignore-immutable")))))
(when (zerop (apply #'majutsu-run-jj args))
(message "Now editing commit %s" revset))))
(if-let* ((revset (majutsu-revision-at-point))
(args (append (list "edit" revset)
(when arg (list "--ignore-immutable")))))
(when (zerop (apply #'majutsu-run-jj args))
(message "Now editing commit %s" revset))
(user-error "No revision at point")))

;;; _
(provide 'majutsu-edit)
Expand Down
30 changes: 29 additions & 1 deletion majutsu-evil.el
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,28 @@ If KEYMAP is not yet bound, defer binding until it becomes available."
majutsu-diff-mode))
(evil-set-initial-state mode majutsu-evil-initial-state))))

(defun majutsu-evil--adjust-section-bindings ()
"Unbind C-j from section maps so Evil navigation takes precedence.
This mirrors `evil-collection-magit-adjust-section-bindings'."
(when (boundp 'majutsu-diff-section-map)
(define-key majutsu-diff-section-map "\C-j" nil))
(when (boundp 'majutsu-file-section-map)
(define-key majutsu-file-section-map "\C-j" nil))
(when (boundp 'majutsu-hunk-section-map)
(define-key majutsu-hunk-section-map "\C-j" nil)))

(defun majutsu-evil--define-mode-keys ()
"Install Evil keybindings for Majutsu maps."
;; Unbind C-j from section maps first.
(majutsu-evil--adjust-section-bindings)
;; Normal/visual/motion share the same bindings for navigation commands.
(majutsu-evil--define-keys '(normal visual motion) 'majutsu-mode-map
(kbd "C-j") #'magit-section-forward
(kbd "C-k") #'magit-section-backward
(kbd "g j") #'magit-section-forward-sibling
(kbd "g k") #'magit-section-backward-sibling
(kbd "]") #'magit-section-forward-sibling
(kbd "[") #'magit-section-backward-sibling
(kbd "R") #'majutsu-restore
(kbd "g r") #'majutsu-refresh
(kbd "`") #'majutsu-process-buffer
Expand Down Expand Up @@ -114,7 +132,17 @@ If KEYMAP is not yet bound, defer binding until it becomes available."
(kbd "Y") #'majutsu-duplicate-dwim)

(majutsu-evil--define-keys '(normal visual) 'majutsu-diff-mode-map
(kbd "g d") #'majutsu-jump-to-diffstat-or-diff)
(kbd "g d") #'majutsu-jump-to-diffstat-or-diff
(kbd "C-<return>") #'majutsu-diff-visit-workspace-file)

;; majutsu-blob-mode is a minor mode, need hook + define-keys
(add-hook 'majutsu-blob-mode-hook #'evil-normalize-keymaps)
(majutsu-evil--define-keys '(normal visual motion) 'majutsu-blob-mode-map
(kbd "p") #'majutsu-blob-previous
(kbd "n") #'majutsu-blob-next
(kbd "q") #'majutsu-blob-quit
;; RET visits the revision (edit)
(kbd "RET") #'majutsu-edit-changeset)

(majutsu-evil--define-keys '(normal visual motion) 'majutsu-log-mode-map
(kbd ".") #'majutsu-log-goto-@
Expand Down
Loading