Re: [[TIG][PATCH] 2/3] Display correct diff the context in split log view

[Jonas Fonseca <fonseca@diku.dk>]

On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah <a.kumar@alumni.iitm.ac.in> wrote:
> In the log view, when scrolling across a commit, the diff view should
> automatically switch to the commit whose context the cursor is on in
> the log view. This commit changes things to catch the REQ_ENTER in the
> log view and handle recalculation of the commit and diff display from
> log_request, rather than delegating it to pager_request. In addition,
> it also gets rid of unexpected upward scrolling of the log view.
>
> Fixes GH #155
>
> Signed-Off-By: Kumar Appaiah <a.kumar@alumni.iitm.ac.in>
> ---
>  NEWS  |  1 +
>  tig.c | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index 0394407..f59e517 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -46,6 +46,7 @@ Bug fixes:
>   - Fix rendering glitch for branch names.
>   - Do not apply diff styling to untracked files in the stage view. (GH #153)
>   - Fix tree indentation for entries containing combining characters. (GH #170)
> + - Introduce a more natural context-sensitive log display. (GH #155)
>
>  tig-1.1
>  -------
> diff --git a/tig.c b/tig.c
> index dd4b0f4..53947b7 100644
> --- a/tig.c
> +++ b/tig.c
> @@ -4478,6 +4478,18 @@ log_request(struct view *view, enum request request, struct line *line)
>                 state->update_commit_ref = TRUE;
>                 return pager_request(view, request, line);
>
> +       case REQ_ENTER:
> +               /* Recalculate the correct commit for the context. */

See my dislike for this type of comments. ;)

> +               state->update_commit_ref = TRUE;
> +
> +               open_view(view, REQ_VIEW_DIFF, OPEN_SPLIT);

This is called every time the user presses up/down. There should be a
check that compares the VIEW(REQ_VIEW_DIFF)->ref to ref_commit.

> +               update_view_title(view);

This can be deleted. pager_request require this hack due to the
automatic scrolling (if I recall correctly).

> +
> +               /* We don't want to delegate this to pager_request,
> +                  since we don't want the extra scrolling of the log
> +                  view. */

This explanation should IMO go into the commit message and not a
comment since it is somewhat confusing unless you are familiar with
the previous behaviour.

> +               return REQ_NONE;
> +
>         default:
>                 return pager_request(view, request, line);

This line can be changed to `return request;`

>         }
> --
> 1.8.3.2
>

-- 
Jonas Fonseca