Magit: Use Unicode box-drawing characters for magit-log

[haxney]

Replacing the current use of back- and forward-slashes, pipes, and asterisks with Unicode box-drawing characters would increase the readability of *magit-log* buffers. This would change the output from this (taken from the git mirror of the Emacs repo):

785f829 | * * alloc.c (mark_object): Use meaningful PVEC_NORMAL_VECTOR. * lisp.h (enum pvec_type): Adjust comments and omit explicit initializer for PVEC_NORMAL_VECTOR.
995a23d | * Clean out old termopts cruft.
1faee51 | *   select.el (xselect--encode-string): If a coding is specified for selection, and that is compatible with COMPOUND_TEXT, use it.
        | |\
360ac10 | | *   merge trunk
        | | |\
        | | |/
        | |/|
81adaff | * | * alloc.c (gc_sweep): Use pointer-to-a-pointer loop for buffers.
15b5649 | | * select.el (xselect--encode-string): If a coding is specified for selection, and that is compatible with COMPOUND_TEXT, use it.
        | |/
e56411e | * In switch-to-buffer optionally restore window point (Bug#4041).
612ea6b | * * commands.h (immediate_quit): Remove duplicate decl.

To this:

785f829 │ ◆ * alloc.c (mark_object): Use meaningful PVEC_NORMAL_VECTOR. * lisp.h (enum pvec_type): Adjust comments and omit explicit initializer for PVEC_NORMAL_VECTOR.
995a23d │ ◆ Clean out old termopts cruft.
1faee51 │ ◆   select.el (xselect--encode-string): If a coding is specified for selection, and that is compatible with COMPOUND_TEXT, use it.
        │ │╲
360ac10 │ │ ◆   merge trunk
        │ │ │╲
        │ │ │╱
        │ │╱│
81adaff │ ◆ │ * alloc.c (gc_sweep): Use pointer-to-a-pointer loop for buffers.
15b5649 │ │ ◆ select.el (xselect--encode-string): If a coding is specified for selection, and that is compatible with COMPOUND_TEXT, use it.
        │ │╱
e56411e │ ◆ In switch-to-buffer optionally restore window point (Bug#4041).
612ea6b │ ◆ * commands.h (immediate_quit): Remove duplicate decl.

This can be done with a few query-replaces.

A side benefit is that commits beginning with * are no longer visually confused with the nodes of the commit graph.

[haxney]

Hmm, it looks like the font on GitHub doesn't make the lines connect. If you kill-ring-save and yank the text into an Emacs buffer, the lines will actually connect. Without that, it is pretty pointless.

I might try my hand at implementing this at some point, but I think it literally would be just a few query-replaces.

[dudebout]

Which font are you using in emacs that gives a nice rendering? Inconsolata does not seem to support these glyphs and in my case emacs uses another font that does not connect the lines.

[pjweisberg]

It looks slightly better in Emacs, but not quite perfect. Just like in Firefox, the ╱ character is the wrong width.

[haxney]

I'm using DejaVu Sans Mono as my font. Here is a screenshot of what it looks like for me:

Here's the output of (describe-face 'default):

Face: default (sample) (customize this face)

Documentation:
Basic default face.

Defined in `faces.el'.

        Family: DejaVu Sans Mono
       Foundry: unknown
         Width: normal
        Height: 83
        Weight: normal
         Slant: normal
    Foreground: black
    Background: white
     Underline: nil
      Overline: nil
Strike-through: nil
           Box: nil
       Inverse: nil
       Stipple: nil
          Font: #<font-object -unknown-DejaVu Sans Mono-normal-normal-normal-*-11-*-*-*-m-0-iso10646-1>
       Fontset: -unknown-DejaVu Sans Mono-normal-normal-normal-*-11-*-*-*-m-0-fontset-startup
       Inherit: nil

[haxney]

It's probably not worth doing if it only works for a specific set of fonts. It could be a boon to use characters other than * before each commit (since the commit itself might start with that character), but without the rest, it's not as much of a benefit.

[dudebout]

Even if it does not look perfect (yet), there is a case to be made for it. It replaces some characters used to look like something (|, \, /, and _) by unicode characters that carry a meaning of drawing. People can then decide to render these unicode characters with the *usual_ ones.

I would encourage at least having the option to turn this on in magit.

[georgek]

I've actually been working on a graph renderer written in emacs lisp which uses unicode box drawing characters. It's at the stage where it's still a WIP but I can't really be bothered to do much else to it unless people are actually interested in integrating it into magit.

It currently looks like this:

The code can be found at https://github.com/georgek/magit-pretty-graph (run (magit-pg-repo "/some/path") where /some/path contains a git repo and check the _magit-prettier-graph_ buffer).

Rendering the graph in emacs as opposed to using git's built in graph might have some advantages, but mainly I think it looks nicer.

[dudebout]

It looks great
On May 6, 2013 9:17 AM, "George Kettleborough" [email protected]
wrote:

I've actually been working on a graph renderer written in emacs lisp which
uses unicode box drawing characters. It's at the stage where it's still a
WIP but I can't really be bothered to do much else to it unless people are
actually interested in integrating it into magit.

It currently looks like this:

[image: emacs]https://f.cloud.github.com/assets/85981/466042/ab49c9d0-b64e-11e2-893b-e4e67a160af3.png

The code can be found at https://github.com/georgek/magit-pretty-graph(run (magit-pg-repo "/some/path") where /some/path contains a git repo and
check the _magit-prettier-graph_ buffer).

Rendering the graph in emacs as opposed to using git's built in graph
might have some advantages, but mainly I think it looks nicer.

—
Reply to this email directly or view it on GitHubhttps://github.com/magit/magit/issues/495#issuecomment-17480757
.

[haxney]

+1

That looks badass

[legumbre]

Very nice!

[ncalexan]

@georgek:

Rendering the graph in emacs as opposed to using git's built in graph might have some advantages, but mainly I think it looks nicer.

Do you mean parsing the logs and reconstructing the parent information in elisp? git itself is very slow to do this on large repositories with many merges (in the worst case, you need to read every commit). Pretty-printing the output of git log --graph is reasonable, but doing this ourselves is going to have abysmal performance.

[georgek]

Do you mean parsing the logs and reconstructing the parent information in elisp?

Yes basically, it runs the git command git log --pretty=format:"%H %P" to get the parents of each commit and matches them up in elisp to build the graph. I don't find any difference in git's performance between this and --graph and I'm not sure why there would be since git would need the same information to draw its own graph. The elisp that draws the graph seems fast enough, git itself is the limiting factor. It might also be an idea to call git asynchronously, but that's another issue.

[sigma]

@georgek I'd certainly integrate this in magit, provided that it can be toggled (ideally if it could be implemented as a minor-mode for magit-log-mode, that would be great).

[ncalexan]

On Mon, May 6, 2013 at 12:12 PM, George Kettleborough <
[email protected]> wrote:

Do you mean parsing the logs and reconstructing the parent information in
elisp?

Yes basically, it runs the git command `git log --pretty=format:"%H %P". I
don't find any difference in git's performance between this and
constructing the graph and I'm not sure why there would be since git would
need the same information to draw its own graph. The elisp that draws the
graph seems fast enough, git itself is the limiting factor. It might also
be an idea to call git asynchronously, but that's another issue.

I would need to see the algorithm, but it appears to me that in the worst
case you must examine every single commit to know the width of the graph.
(Imagine n merges.) git itself is slow to draw the graph in this case
because it needs to page a huge amount of commit info through the disk
cache, but I have faith that git is more performant than any elisp we can
write.

[georgek]

I don't know how git does it internally, but once the list of hashes are in emacs it takes just a single pass through them to draw the graph. You certainly don't need to examine every commit in the history. It works for any number of commits in an interval. Try running it, but I think the performance is acceptable.

I haven't actually tested it with an octopus merge (although it should work). I've been testing it with real repos like magit and git. The git repo actually looks pretty terrible, but then it does with git log --graph too (too many branches). Maybe we can make some worst case repos to test it with?

[haxney]

Some thoughts on the code

Code

Compile errors

There are some issues with compiling and running it. Loading the file directly results in the error

let*: Symbol's function definition is void: dolistc

I ran this with emacs -Q then (load-file "/path/to/magit-pretty-graph.el") and got the following:

Debugger entered--Lisp error: (void-function dolistc)
  (dolistc (c (cdr l)) (setcar c (1+ ll)) (setq ll (car c)) (if (null (cdr c)) (progn (setcdr c l) (return l))))
  (let* ((l (make-list length 1)) (ll (car l))) (dolistc (c (cdr l)) (setcar c (1+ ll)) (setq ll (car c)) (if (null (cdr c)) (progn (setcdr c l) (return l)))))
  magit-pg-make-ring(5)
  (defconst magit-pg-colour-ring (magit-pg-make-ring 5))
  eval-buffer(#<buffer  *load*> nil "/tmp/magit-pretty-graph.el" nil t)  ; Reading at buffer position 3730
  load-with-code-conversion("/tmp/magit-pretty-graph.el" "/tmp/magit-pretty-graph.el" nil nil)
  load("/tmp/magit-pretty-graph.el" nil nil t)
  load-file("/tmp/magit-pretty-graph.el")
  eval((load-file "/tmp/magit-pretty-graph.el") nil)
  eval-last-sexp-1(t)
  eval-last-sexp(t)
  eval-print-last-sexp()
  call-interactively(eval-print-last-sexp nil nil)

On Emacs 24.3.1 under Linux. Byte-compiling and then loading gives the following error:

Debugger entered--Lisp error: (void-function c)
  c((1 1 1 1))
  magit-pg-make-ring(5)
  (defconst magit-pg-colour-ring (magit-pg-make-ring 5))
  load("/tmp/magit-pretty-graph.elc" nil nil t)
  load-file("/tmp/magit-pretty-graph.elc")
  eval((load-file "/tmp/magit-pretty-graph.elc") nil)
  eval-last-sexp-1(t)
  eval-last-sexp(t)
  eval-print-last-sexp()
  call-interactively(eval-print-last-sexp nil nil)

To get it to work, I have to eval-defun the definition of dolistc and then eval-buffer.

CL requirements

Use car rather than first.

dolistc

The byte compiler really doesn't like it.

Performance

Scrolling up and down with previous-line and next-line causes the CPU to spike if the *magit-prettier-graph* buffer is not the selected one. This doesn't occur in emacs -Q, so I'm not exactly sure what part of my setup is causing it. There don't seem to be an unreasonable number of overlays, so I'm not sure what it is.

Appearance

The addition of colors is a huge improvement over my earlier suggestion. I wonder if there aren't better characters to use for some of the elements; the commit marker in particular. They aren't that visually distinct. Looking at the Unicode box-drawing chart, there don't seem to be any better box-drawing options, and other characters wouldn't preserve the box-ness of the display.

As inspiration, consider gitk. It is included in the core Git distribution, and from my (admittedly brief) exploration of alternatives, I haven't found any other visualization I like nearly as much. The problem is that, with the tools available, we can't draw dots on top of lines, so you are left with what I had in my earlier suggestion, which is fugly.

[haxney]

Here's a thought as far as images: use a BLACK DIAMOND for commits after the BOX DRAWINGS VERTICAL LIGHT AND RIGHT HEAVY. It would look something like this:

(I edited this manually, so some things might not be colored correctly or line up)

[georgek]

Ah good idea with the diamond, it looks much better now (see latest commit). It might look better with WHITE DIAMOND but this can be decided later I think.

Regarding gitk etc. there are certain restrictions with only being able to use vertical and horizontal lines. gitk and git log --graph use diagonal lines so that they can push existing branches out to the right. Without diagonals this is not possible without leaving lots of extra space between branches and it looks bad anyway.

gitk does do something interesting to reduce the number of branches displayed, though (I call these "trunks" in the code btw). It seems that if it detects a very long trunk with no commits (which happens if an old commit is cherry picked for example) it breaks it and draws arrows linking them up. This might actually be possible to do with this but I haven't investigated that yet.

I tried byte-compiling after removing the uses of first etc. and changing set-difference to cl-set-difference it compiles fine.

[haxney]

You might also want to consider using BOX DRAWINGS VERTICAL SINGLE AND RIGHT DOUBLE with either BACK DIAMOND or WHITE DIAMOND as bullet points. Compare the following:

┝◇
╞◇

┝◆
╞◆

This is bikeshedding in the most extreme sense, but it's worth considering the range of symbols available.

[afrepues]

@haxney well, bigger bullets make it easier to spot the commit in the log, and it can get quite intricate.

[tarsius]

Emacs supports the svg image format which could be used to draw graphs as nice as those of gitk, and better.

@yhvh used to have a repository that demonstrated one way this could be used: drawing in an emacs buffer - real graphical drawing, not something like artist-mode. Unfortunately that repository no longer exists. But he also has an emacs package called svg-go which features an interactive go board. The interesting thing to me is that you can manipulate the underlying xml and the graphical display is automatically updated (or at least it appears to work that way).

[georgek]

Doing graphics via svg seems a bit inefficient to me, especially considering the verbosity of xml. Is it not possible to get at the graphical part of emacs directly?

I'm not sure how I feel about doing pixel based graphics in emacs, though. The unicode stuff can at least work over ssh etc. without X11 forwarding. Graphics seems a bit too close to unhackable GUI IDEs to me.

[tarsius]

I only mentioned this as another option. Please push ahead with our idea, it looks lovely.

[georgek]

I had a go at getting this to work with magit's log then I ran out of time to work on this any longer. I wonder if someone else could advise have a look and figure out how best to integrate this into magit. At the moment my thing gets git's output in one buffer and then draws the graph in another buffer. But I guess it could be made to "wash" the output just in one buffer.

It's easy to build up a list of magit-log-line objects and then pass them to magit-present-line-function. I guess the only difference between the two is how these magit-log-line objects are created, after that making the overlays and author-date thing should be all the same. I just ended up going around in circles trying to work out how to do it with not too much code duplication, though.

[afrepues]

@georgek:

I had a go at getting this to work with magit's log then I ran out of time to work on this any longer. I wonder if someone else could advise have a look and figure out how best to integrate this into magit.

Is the code in the same place?

At the moment my thing gets git's output in one buffer and then draws the graph in another buffer. But I guess it could be made to "wash" the output just in one buffer.

I'd prefer simpler, easier to read and maintain code over having one buffer only, the saving in-place rewriting might provide can be achieved by removing lines from the git output buffer as soon as they are no longer needed.

It's easy to build up a list of magit-log-line objects and then pass them to magit-present-line-function. I guess the only difference between the two is how these magit-log-line objects are created, after that making the overlays and author-date thing should be all the same.

This sound like a good strategy.

I just ended up going around in circles trying to work out how to do it with not too much code duplication, though.

This can worked out later, I wouldn't worry too much about it right now.

[georgek]

The recent changes to the log line stuff makes it easier to do this. I've now done a hack which replaces the oneline style with my style. You can try it by getting this branch of magit: https://github.com/georgek/magit/tree/pretty-graph and updating the pretty graph stuff from: https://github.com/georgek/magit-pretty-graph Eval magit-pretty-graph.el before running magit-log

The changes to magit are:

1. It uses a very different command for git. The command currently doesn't change according to options.
2. The function magit-pg-2 is run on the log buffer. This is what draws the graph. The buffer is left in a state that is very easy to parse by magit-wash-log-line.
3. The regex to parse the lines is changed to match the output of magit-pg-2.

It's not actually necessary to use regex at all for this way of drawing the graph. I get the output for git with the bits separated by null characters (^@) so the bits can be found simply with split-string. So using magit-wash-log-line for this is silly, but I did this to avoid making a big change now. If there is still interest in this then we can work towards getting this working properly.

[tarsius]

I have implemented the simple variant as originally suggested by @haxney. The reason this has taken so long even though I have added option magit-log-format-graph-function a long time ago, was that I was waiting for the rest to be contributed by somebody else. By the time I realized that was not gonna happen I had already lost my basic implementation and wasn't motivated to write it again. (It was probably fairly similar to what I have committed now, but I did also dive into the git C code making sure that I did not miss any character that might appear in git's output in rare situations. I haven't done that again - probably o is the only such character anyway (octopus merge)).

An other reason is that I don't really think this looks any better - depending on the font it might also luck much worse. So no, I won't enable that by default.

As for @georgek variant, I still really like the look of it, and was holding out for a an "algorithm-only" version. I would like to do the integration into the existing logging code myself, especially because that is still under construction, but also because I know the pitfalls. So this is most likely not going to make it into 2.1.0.

@georgek What I mean by a "algorithm-only version" is that there should a function to which I can pass a list (HASH PARENT-HASH...) and which then returns (GRAPH-STRING...). The function can be a blackbox that uses some variables to do its state tracking but it should not attempt to actually render any log entries, and instead limit itself to just the graph part.

[georgek]

I did want to provide a black box as you describe but I couldn't find the interface in the magit code last time I looked.

The interface needs to be more than just the hash and parent hashes. I think at least an extra output is necessary. The reason for this is that the calling function will have no idea whether the graph line returned is really for that commit, or some "decoration" around the commit (ie. branches and merges). I think all of the "decoration" can be drawn before the next commit, so each time the function is called it would first return lines for the decoration and then finally return the commit itself.

Another problem is with the "long" graph format. The function needs to be asked for spacing between commits and decoration, and therefore I think an extra input is needed too (or another function).

I'll need to think about what kind of interface is necessary properly, though. If we can agree on an interface and meet in the middle that would be great. It would also mean other people can make their own graph drawings too.

[tarsius]

Yes, let's do that. I'll get back to this soon.

[tarsius]

I did want to provide a black box as you describe but I couldn't find the interface in the magit code last time I looked.

There was none but I have now create such an interface. It's rather ugly and likely will have to change, but it's a start. See https://github.com/tarsius/magit/tree/pretty-graph.

The interface needs to be more than just the hash and parent hashes. I think at least an extra output is necessary.

Yes, of course. I should probably have mentioned that I was aware of that when I suggested the simplified interface. The requirements you mention make sense to me too.

I think all of the "decoration" can be drawn before the next commit

I overlooked this and so my draft also contains support for graph lines before the star line. Maybe that's not so bad, as you mentioned other might want to write their own variant, and those implementations might need this.

But it would be great if the star line always were the first line, otherwise I would have to do some read-ahead on my side.

Another problem is with the "long" graph format. The function needs to be asked for spacing between commits and decoration,

I don't quite understand what you are saying here, but assume you are probably talking about the following.

For long logs we don't know how many lines are required to show a commit's message etc. E.g. there might be more content lines than graph lines, or vice-versa. I think you are suggesting that your entry function should be informed about this. I think it would be better if the value returned by it included information about "if you need to fill more lines, then repeat this line".

The branch linked to above does implement the interface as needed from my side for both oneline and long logs, but then only uses it for oneline logs. Using your pretty logs for long logs would require more work on my side but I don't think that implementing that would (as far as the existing log code is concerned) require any changes to the interface. Actually I have already taken these future needs into account when I wrote this suggested interface, it includes things not needed for oneline log.

I also intend to improve long log in ways not related to the look of the graphs. Commit sections in a long log could have subsections just like a commit displayed in its own buffer. This would be useful when showing diffs inline, which in turn would be useful when using log options such as -G and -L. So it is possible to tell your code how many lines it has to provide the graph part for, that isn't fixed. Also I would like to implement this later.

So here is an example of what I suggest your function should consume and return:

(magit-log-pretty-graph COMMIT PARENT...)
=> (;; The graph string at position 2 is to be aligned with the
    ;; commit's "mainline".
    2
    ;; The graph strings at positions (abs -1) and (abs -3) can be
    ;; repeated to fill more lines they can be omitted if there are
    ;; less content lines than graph lines.  If these are positive
    ;; numbers that means that they are required at least once.
    -1
    -3
    ;; And finally the graph strings.
    "|\\" ; 0 required
    "| |" ; 1 zero or more times
    "| *" ; 2 the star line
    "| |" ; 3 zero or more times
    "|/") ; 4 required

Of course this could also look like:

((1 "|\\" "| |") "| *" (0 "| |" "|/"))

[tarsius]

@georgek Do you intend to work on this again? The reason that I have not worked on this myself is that I find your code very hard to read. I have to be bold here to avoid the two of us waisting any time: I think this has to be rewritten from scratch.

A good place to start would be to use defstruct, this would help avoiding the setcar, setcdr, nth etc. mess going on. Then avoid the temptation to write your own macros. magit-pg-dolistc seems like a particularly bad idea; such looping macros should abstract the task of walking over a list; this seems to do the opposite by forcing you to constantly use setcar and setcdr. It is better to use dolist, while, or any of the map*** variants _as appropriate_, instead of writing your own variant of dolist and then use it for _everything_. Try to use less variables and side-effects, and generally a more functional style. E.g instead of let and magit-pg-stradd use just concat outmost; and instead of magit-pg-dolistc and magit-pg-stradd use mapconcat. Write shorter functions that do one thing well. Get the core functionality right and clean before doing non-essential things like defining faces.

If you mostly agree with these suggestions, then I think we should push this further. On the other hand if you thing I am exaggerating and/or we just prefer a different coding styles, then I think it is best to close this issue. It has been in limbo for over a year now and if the reason for that was that we both were just waiting for the other side, then there really is no point.

Sorry for being bold, I hope it's for the best. Jonas

[georgek]

I agree with it being rewritten. This is why I haven't worked on it because unfortunately I cannot find the time right now. I appreciate the advice and agree with you. I'd love to do it properly, but I can't say when that might be yet.

[tarsius]

Uuuf, I was a bit worried you would take it the wrong way. I shouldn't write stuff like the above, right in the middle of a sleep cycle adjustment... No, problem if you need time... better that way actually. I need time for other things too, so let's postpone this until after 2.1.0.

[tarsius]

The issue this originally was about has been addressed. Because of that and because I have opened a new issue (#1425) which also takes into account --graphs performance issues I am now closing this.

[Pitometsu]

Should I use for such nice unicode https://github.com/georgek/magit-pretty-graph or is it already part of magit, and how can I customize it then?

[tarsius]

Magit doesn't currently support unicode graphs and loading magit-pretty-graph will certainly break Magit.

I plan to implement prettier graphs eventually, see #2989.

[georgek]

Actually it won't break magit because it calls git directly. The problem is it's not a part of magit at all. It's an emacs rendering of the git graph.

I actually have time to work on this now if you would like to, @tarsius. It might at least save you some effort in reinventing what I did before.

[tecosaur]

Hey @georgek, this looks lovely! Do you plan on getting back to this eventually? (assuming that @tarsius is still interested :slightly_smiling_face:)