Fill gaps in dwim behavior

[tarsius]

One of Magit's most important meta-features, is that one usually has to press just one or two key to (1) show another representation/details of the thing at point in another buffer, or (2) perform some action on that thing at point.

In Magit buffers the "thing at point" is usually called the "current section", and it represents some entity or concept that is meaningful to Git. Examples of such entities include commits and files and examples of concepts include the lists (logs) of unpushed and unpulled commits.

Dwim behavior exists both for sections that represent entities as well as for those that represent concepts. For example RET on a commit in a log shows that commit in another buffer (like git show), while doing the same on the heading "Unpushed to origin/master" shows the diff for <upstream>..<current-branch>.

RET is the most important dwim command, it does something meaningful for almost all sections. But there are other commands, including, but not limited to, keys that invoke variations of this "most-dwim" command. E.g. inside a hunk C-return always jumps to the file, while RET may jump to a blob instead, depending on what the diff represents.

Knowing exactly what some section represents is another, closely related, meta-feature of Magit. This sounds rather obvious, but keep in mind that with Git, for example, a diff (and everything else) is basically just text. It is you who has to remember what command you typed to generate it, and what actions you can therefor perform on it (in Git e.g. by piping it into some other command). Magit on the other hand remembers this for you and if you invoke some command that actually performs different but closely related actions based on context, then that command automatically performs the appropriate variation.

In a sense every command (in every program) is a dwim command - if it doesn't do what you mean, than it is a command that either has a bug or that is insufficiently documented (or if you don't read documentation, a command that does not conform to your intuition).

In Magit I find it difficult to draw the line between dwim commands and other commands that just happen to do something useful and to have a mnemonic key binding. So maybe it is more useful when explaining what makes Magit different from other user interfaces, to not focus on it having many dwim commands, but to instead say that you can (1) act on everything that you can see, that you can (2) directly do (almost) everything you might wish to do, and (3) that you can do so using highly consistent and mnemonic key bindings.

---

To cut a long story short, there are gaps in the dwim behavior. I should have kept a list, but unfortunately I haven't. On the other hand I have in the past filled most gaps that were reported by users rather quickly, so there might not be that many after all.

To find the remaining gaps I should probably start by creating a list of all the "paths" that do make sense and then check which of those haven't actually been implemented yet.

Thinking of it, such a list would be very useful as documentation, not just to find gaps. And while a list is a good start, I should go further and create a visual guide, consisting of screenshots of the various Magit and related buffers and arrows connecting them (with associated key bindings etc). I could create a poster from that, and maybe sell it.

---

But for now, there's what I can find at this moment:

(Some of these might stretch the definition of "dwim behavior" a bit.)

• Show details/other aspect in another buffer.

  • Jumping from a diff to a file/blob, is already well supported. But the opposite is not, and that is just as useful. I would say this is the biggest gap that still exists, and one of the few that have been known for a long while without getting stuffed.

    I.e. I have been planning to do something about this since long before this issue was opened: Jump to unstaged hunk corresponding to line at point  #2968 "Jump to unstaged hunk corresponding to line at point". However that still added something new, I was planning to support jumping to a dedicated diff buffer, but jumping to the diff inside the status buffer is also useful. The code presented there however is not useful.

    Actually this is already supported, but only when the file/blob buffer contains blame information and even then, this does not jump to the correct location inside the diff.

  • When blaming, allow showing the revision before the revision at point.

  • Given a commit, show log of "surrounding" commits #2990 Given a commit, show log of "surrounding" commits.

  • Linkify issues and hashes in commit messages #2949 Linkify issues and hashes in commit messages.

  • Make SPC scroll blob/file buffer while point is on diff #2833 Make SPC scroll blob/file buffer while point is on diff.

• Show/expand details in current buffer.

  • Allow selectively showing more context in diffs #2953 Allow selectively showing more context in diffs.
  • Show diff in file-visiting buffer #2952 Show diff in file-visiting buffer.
  • Diffs should optionally support syntax highlighting #2942 Diffs should optionally support syntax highlighting.
  • Provide alternative visualizations of blame information #2937 Provide alternative visualizations of blame information.

• With side-effects.

  (I should probably open a new issue "New ways of editing" for these.)

  • Make blobs editable, modify containing commit on save #2944 Make blobs editable, modify containing commit on save.
  • Edit the commit that last touched the line at point #2943 Edit the commit that last touched the line at point.
  • Make it possible to edit diff #2938 Make it possible to edit diff.
  • Keep buffers editable when blaming #2936 Keep buffers editable when blaming.

[tarsius]

The code that calculates position offsets by taking relevant diffs into account has to be refactored. It is unreadable and therefore hard to adjust for other use cases.

This is what has prevented me from implementing #3130. Also once that is done, then magit-log-buffer-file and magit-log-trace-definition can be taught about uncommitted changes (see #3796).

[tarsius]

The code that calculates position offsets by taking relevant diffs into account has to be refactored. It is unreadable and therefore hard to adjust for other use cases.

Turns out it wasn't that bad.

Thanks to #3860 to most glaring holes have been stuffed. And I am now going back to doing the same for new holes as they are being reported. Some linked diff-related topics listed above have not been taken care of yet, but I am closing this anyway because not closely enough related to what I had in mind when I originally opened this. The posters are not ready yet either.

[medranocalvo]

Dear @tarsius,

thank you for all these improvements.

I couldn't find the implementation of #2968 in your goto changes. Please, let me know whether I missed it. Otherwise, please find below a hacky implementation relying on your improvements:

  (defun magit-status-here ()
    "Jump to hunk corresponding to current line in magit."
    (interactive)
    (let ((filename (file-truename (buffer-file-name)))
          (line (line-number-at-pos))
          (col (current-column)))
      (call-interactively #'magit-status)
      (when filename                    ; Guard against non-file-visiting buffers.
        (let* ((filename (magit-file-relative-name filename))
               (unstaged-section (magit-get-section '((unstaged) (status))))
               (file-section (cl-find-if (lambda (subsection)
                                            (and (eq (oref subsection type) 'file)
                                                 (equal (oref subsection value) filename)))
                                          (oref unstaged-section children))))
          (when file-section
            (magit-section-show file-section)
            (let ((magit-root-section unstaged-section))
              (magit-diff--goto-position filename line col))
            (recenter))))))

The gross hack is binding magit-root-section; it would be better to generalize magit-diff--goto-position w.r.t. the root diff section.

[tarsius]

@medranocalvo I'll do that too, maybe next week. But it's a bit more complicated than that. Going to the unstaged changes is not correct in some cases and we have to go to the staged changes instead.

[medranocalvo]

Dear @tarsius, congratulations for the excellent work and thank you again for your generosity. Let me know if I can help in any way.

[tarsius]

👋 I have implemented this command a while ago in #3930.

[medranocalvo]

Dear @tarsius,
finally had the chance to test it. It works wonderfully, thank you very much!
Would it make sense to ###autoload it?
Regards,
Adrián.

[tarsius]

It's autoloaded now. Seems like an oversight it wasn't autoloaded initially.