{"thread":{"id":"34593","subject":"[[TIG][PATCH] 2/3] Display correct diff the context in split log view","startedAt":"2013-08-03T00:23:16Z","lastAt":"2013-08-06T04:08:26Z","messageCount":9,"participants":["Kumar Appaiah","Jonas Fonseca"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"224476","messageId":"1375489399-11618-1-git-send-email-a.kumar@alumni.iitm.ac.in","threadId":"34593","inReplyTo":null,"subject":"[TIG][PATCH 0/3] Refactoring of the log view","fromName":"Kumar Appaiah","fromEmail":"a.kumar@alumni.iitm.ac.in","sentAt":"2013-08-03T00:23:16Z","receivedAt":"2013-08-03T00:23:16Z","isPatch":true,"sender":{"key":"a.kumar@alumni.iitm.ac.in","avatar":"https://gravatar.com/avatar/7ce299108bfff26792729bde4a1dd0638292f852a042d40a0a1991ff2ae58b0b?d=mp&s=160"},"body":"Hi.\n\nThese set of patches refactor the log view to provide a behaviour that\nis quite similar to, say, e-mail with Mutt. The key improvements are:\n\n- The current commit is inferred based on the context. For example, if\n you focus on the commit message of a particular commit, the correct\n commit is inferred automagically.\n\n- Scrolling the log view when the diff is open shows the correct\n commit on the screen, rather than have to scroll up and cross the\n commit line to display the screen.\n\nI have decided to revert 888611dd5d407775245d574a3dc5c01b5963a5ba,\nsince the behaviour with the updated scrolling pattern is much more\nconsistent.\n\nAs always, I will gladly alter the patch based on comments on coding\nstyle and all other aspects.\n\nThanks!\n\nKumar\n\nKumar Appaiah (3):\n Add log_select function to find commit from context in log view\n Display correct diff the context in split log view\n Revert \"Scroll diff with arrow keys in log view\"\n\n NEWS | 1 +\n tig.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 63 insertions(+), 5 deletions(-)\n\n-- \n1.8.3.2\n"},{"id":"224478","messageId":"1375489399-11618-2-git-send-email-a.kumar@alumni.iitm.ac.in","threadId":"34593","inReplyTo":"1375489399-11618-1-git-send-email-a.kumar@alumni.iitm.ac.in","subject":"[[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view","fromName":"Kumar Appaiah","fromEmail":"a.kumar@alumni.iitm.ac.in","sentAt":"2013-08-03T00:23:17Z","receivedAt":"2013-08-03T00:23:17Z","isPatch":true,"sender":{"key":"a.kumar@alumni.iitm.ac.in","avatar":"https://gravatar.com/avatar/7ce299108bfff26792729bde4a1dd0638292f852a042d40a0a1991ff2ae58b0b?d=mp&s=160"},"body":"This commit introduces and uses the log_select function to find the\ncorrect commit in the unsplit log view. In the log view, if one\nscrolls down across a commit line, the current commit (as displayed in\nthe status bar) gets updated, but not so when scrolling upward across\na commit. The log_select function handles this scenario to to the\n``right thing''. In addition, it introduces the log_state structure as\nthe private entry of the log view to hold a flag that decides whether\nto re-evaluate the current commit based on scrolling.\n\nSigned-off-by: Kumar Appaiah \n---\n tig.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 48 insertions(+), 2 deletions(-)\n\ndiff --git a/tig.c b/tig.c\nindex 72f132a..dd4b0f4 100644\n--- a/tig.c\n+++ b/tig.c\n@@ -4384,6 +4384,33 @@ pager_select(struct view *view, struct line *line)\n \t}\n }\n \n+struct log_state {\n+\tbool update_commit_ref;\n+};\n+\n+static void\n+log_select(struct view *view, struct line *line)\n+{\n+\tstruct log_state *state = (struct log_state *) view->private;\n+\n+\tif (state->update_commit_ref && line->lineno > 1) {\n+\t\t/* We need to recalculate the previous commit,\n+\t\t since the user has likely scrolled up. */\n+\t\tconst struct line *commit_line = find_prev_line_by_type(view, line, LINE_COMMIT);\n+\n+\t\tif (commit_line)\n+\t\t\tstring_copy_rev(view->ref, (char *) (commit_line->data + STRING_SIZE(\"commit \")));\n+\t}\n+\tif (line->type == LINE_COMMIT) {\n+\t\tchar *text = (char *)line->data + STRING_SIZE(\"commit \");\n+\n+\t\tif (!view_has_flags(view, VIEW_NO_REF))\n+\t\t\tstring_copy_rev(view->ref, text);\n+\t}\n+\tstring_copy_rev(ref_commit, view->ref);\n+\tstate->update_commit_ref = FALSE;\n+}\n+\n static bool\n pager_open(struct view *view, enum open_flags flags)\n {\n@@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)\n static enum request\n log_request(struct view *view, enum request request, struct line *line)\n {\n+\tstruct log_state *state = (struct log_state *) view->private;\n+\n \tswitch (request) {\n \tcase REQ_REFRESH:\n \t\tload_refs();\n \t\trefresh_view(view);\n \t\treturn REQ_NONE;\n+\n+\tcase REQ_MOVE_UP:\n+\tcase REQ_PREVIOUS:\n+\t\tif (line->type == LINE_COMMIT && line->lineno > 1) {\n+\t\t\t/* We are at a commit, and heading upward. We\n+\t\t\t force log_select to find the previous\n+\t\t\t commit above, from the context. */\n+\t\t\tstate->update_commit_ref = TRUE;\n+\t\t}\n+\t\treturn pager_request(view, request, line);\n+\n+\tcase REQ_MOVE_PAGE_UP:\n+\tcase REQ_MOVE_PAGE_DOWN:\n+\t\t/* We need to figure out the right commit again. */\n+\t\tstate->update_commit_ref = TRUE;\n+\t\treturn pager_request(view, request, line);\n+\n \tdefault:\n \t\treturn pager_request(view, request, line);\n \t}\n@@ -4441,13 +4487,13 @@ static struct view_ops log_ops = {\n \t\"line\",\n \t{ \"log\" },\n \tVIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER | VIEW_NO_PARENT_NAV,\n-\t0,\n+\tsizeof(struct log_state),\n \tlog_open,\n \tpager_read,\n \tpager_draw,\n \tlog_request,\n \tpager_grep,\n-\tpager_select,\n+\tlog_select,\n };\n \n struct diff_state {\n-- \n1.8.3.2\n"},{"id":"224475","messageId":"1375489399-11618-3-git-send-email-a.kumar@alumni.iitm.ac.in","threadId":"34593","inReplyTo":"1375489399-11618-1-git-send-email-a.kumar@alumni.iitm.ac.in","subject":"[[TIG][PATCH] 2/3] Display correct diff the context in split log view","fromName":"Kumar Appaiah","fromEmail":"a.kumar@alumni.iitm.ac.in","sentAt":"2013-08-03T00:23:18Z","receivedAt":"2013-08-03T00:23:18Z","isPatch":true,"sender":{"key":"a.kumar@alumni.iitm.ac.in","avatar":"https://gravatar.com/avatar/7ce299108bfff26792729bde4a1dd0638292f852a042d40a0a1991ff2ae58b0b?d=mp&s=160"},"body":"In the log view, when scrolling across a commit, the diff view should\nautomatically switch to the commit whose context the cursor is on in\nthe log view. This commit changes things to catch the REQ_ENTER in the\nlog view and handle recalculation of the commit and diff display from\nlog_request, rather than delegating it to pager_request. In addition,\nit also gets rid of unexpected upward scrolling of the log view.\n\nFixes GH #155\n\nSigned-Off-By: Kumar Appaiah \n---\n NEWS | 1 +\n tig.c | 12 ++++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/NEWS b/NEWS\nindex 0394407..f59e517 100644\n--- a/NEWS\n+++ b/NEWS\n@@ -46,6 +46,7 @@ Bug fixes:\n - Fix rendering glitch for branch names.\n - Do not apply diff styling to untracked files in the stage view. (GH #153)\n - Fix tree indentation for entries containing combining characters. (GH #170)\n+ - Introduce a more natural context-sensitive log display. (GH #155)\n \n tig-1.1\n -------\ndiff --git a/tig.c b/tig.c\nindex dd4b0f4..53947b7 100644\n--- a/tig.c\n+++ b/tig.c\n@@ -4478,6 +4478,18 @@ log_request(struct view *view, enum request request, struct line *line)\n \t\tstate->update_commit_ref = TRUE;\n \t\treturn pager_request(view, request, line);\n \n+\tcase REQ_ENTER:\n+\t\t/* Recalculate the correct commit for the context. */\n+\t\tstate->update_commit_ref = TRUE;\n+\n+\t\topen_view(view, REQ_VIEW_DIFF, OPEN_SPLIT);\n+\t\tupdate_view_title(view);\n+\n+\t\t/* We don't want to delegate this to pager_request,\n+\t\t since we don't want the extra scrolling of the log\n+\t\t view. */\n+\t\treturn REQ_NONE;\n+\n \tdefault:\n \t\treturn pager_request(view, request, line);\n \t}\n-- \n1.8.3.2\n"},{"id":"224477","messageId":"1375489399-11618-4-git-send-email-a.kumar@alumni.iitm.ac.in","threadId":"34593","inReplyTo":"1375489399-11618-1-git-send-email-a.kumar@alumni.iitm.ac.in","subject":"[[TIG][PATCH] 3/3] Revert \"Scroll diff with arrow keys in log view\"","fromName":"Kumar Appaiah","fromEmail":"a.kumar@alumni.iitm.ac.in","sentAt":"2013-08-03T00:23:19Z","receivedAt":"2013-08-03T00:23:19Z","isPatch":true,"sender":{"key":"a.kumar@alumni.iitm.ac.in","avatar":"https://gravatar.com/avatar/7ce299108bfff26792729bde4a1dd0638292f852a042d40a0a1991ff2ae58b0b?d=mp&s=160"},"body":"This reverts commit 888611dd5d407775245d574a3dc5c01b5963a5ba. This is\nbecause, in the re-engineered log view, scrolling the log with the\narrows now updates the diff in the diff view when the screen is\nsplit. This resembles the earlier behaviour, and is also what users of\nsoftware like Mutt (which uses the pager view concept) would expect.\n\nSigned-Off-By: Kumar Appaiah \n\nConflicts:\n\ttig.c\n---\n tig.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/tig.c b/tig.c\nindex 53947b7..65c91a0 100644\n--- a/tig.c\n+++ b/tig.c\n@@ -1901,7 +1901,6 @@ enum view_flag {\n \tVIEW_STDIN\t\t= 1 << 8,\n \tVIEW_SEND_CHILD_ENTER\t= 1 << 9,\n \tVIEW_FILE_FILTER\t= 1 << 10,\n-\tVIEW_NO_PARENT_NAV\t= 1 << 11,\n };\n \n #define view_has_flags(view, flag)\t((view)->ops->flags & (flag))\n@@ -3774,7 +3773,7 @@ view_driver(struct view *view, enum request request)\n \n \tcase REQ_NEXT:\n \tcase REQ_PREVIOUS:\n-\t\tif (view->parent && !view_has_flags(view->parent, VIEW_NO_PARENT_NAV)) {\n+\t\tif (view->parent) {\n \t\t\tint line;\n \n \t\t\tview = view->parent;\n@@ -4498,7 +4497,7 @@ log_request(struct view *view, enum request request, struct line *line)\n static struct view_ops log_ops = {\n \t\"line\",\n \t{ \"log\" },\n-\tVIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER | VIEW_NO_PARENT_NAV,\n+\tVIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER,\n \tsizeof(struct log_state),\n \tlog_open,\n \tpager_read,\n-- \n1.8.3.2\n"},{"id":"224638","messageId":"CAFuPQ1KkUn5t54BXLTnYUcH_jY-SiSEJx3dDVzQ3FpswhFg0Bw@mail.gmail.com","threadId":"34593","inReplyTo":"1375489399-11618-2-git-send-email-a.kumar@alumni.iitm.ac.in","subject":"Re: [[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2013-08-06T03:27:44Z","receivedAt":"2013-08-06T03:27:44Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote:\n> This commit introduces and uses the log_select function to find the\n> correct commit in the unsplit log view. In the log view, if one\n> scrolls down across a commit line, the current commit (as displayed in\n> the status bar) gets updated, but not so when scrolling upward across\n> a commit. The log_select function handles this scenario to to the\n\ns/to to/to do/\n\n> ``right thing''. In addition, it introduces the log_state structure as\n> the private entry of the log view to hold a flag that decides whether\n> to re-evaluate the current commit based on scrolling.\n>\n> Signed-off-by: Kumar Appaiah \n> ---\n> tig.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--\n> 1 file changed, 48 insertions(+), 2 deletions(-)\n>\n> diff --git a/tig.c b/tig.c\n> index 72f132a..dd4b0f4 100644\n> --- a/tig.c\n> +++ b/tig.c\n> @@ -4384,6 +4384,33 @@ pager_select(struct view *view, struct line *line)\n> }\n> }\n>\n> +struct log_state {\n> + bool update_commit_ref;\n> +};\n> +\n> +static void\n> +log_select(struct view *view, struct line *line)\n> +{\n> + struct log_state *state = (struct log_state *) view->private;\n> +\n> + if (state->update_commit_ref && line->lineno > 1) {\n> + /* We need to recalculate the previous commit,\n> + since the user has likely scrolled up. */\n\nI'd prefer that state->update_commit_ref is given another name so it\nwon't be necessary to have these comments everywhere the field is\nused, for example recalculate_commit_context. The comment could be\nmoved to the declaration in struct log_state to explain its use.\n\nMulti-line comments should start with '*' for each additonal line, e.g.\n\n /* bla bla\n * bla bla */\n\n> + const struct line *commit_line = find_prev_line_by_type(view, line, LINE_COMMIT);\n> +\n> + if (commit_line)\n> + string_copy_rev(view->ref, (char *) (commit_line->data + STRING_SIZE(\"commit \")));\n\nYou mentioned elsewhere that this looked funny, and I guess you are\nright. I will extract this into a utility method so you can simply\ncall: string_copy_rev_from_line(view->ref, commit_line); ...\n\n> + }\n> + if (line->type == LINE_COMMIT) {\n> + char *text = (char *)line->data + STRING_SIZE(\"commit \");\n> +\n> + if (!view_has_flags(view, VIEW_NO_REF))\n> + string_copy_rev(view->ref, text);\n\n... and: string_copy_rev_from_line(view->ref, line);\n\n> + }\n> + string_copy_rev(ref_commit, view->ref);\n> + state->update_commit_ref = FALSE;\n> +}\n> +\n> static bool\n> pager_open(struct view *view, enum open_flags flags)\n> {\n> @@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)\n> static enum request\n> log_request(struct view *view, enum request request, struct line *line)\n> {\n> + struct log_state *state = (struct log_state *) view->private;\n\nThere's no need to cast view->private here.\n\n> +\n> switch (request) {\n> case REQ_REFRESH:\n> load_refs();\n> refresh_view(view);\n> return REQ_NONE;\n> +\n> + case REQ_MOVE_UP:\n> + case REQ_PREVIOUS:\n> + if (line->type == LINE_COMMIT && line->lineno > 1) {\n> + /* We are at a commit, and heading upward. We\n> + force log_select to find the previous\n> + commit above, from the context. */\n\nPlease delete this comment.\n\n> + state->update_commit_ref = TRUE;\n> + }\n> + return pager_request(view, request, line);\n\nThere's not really any reason to call pager_request here, since it\nonly handles REQ_ENTER.\n\n> +\n> + case REQ_MOVE_PAGE_UP:\n> + case REQ_MOVE_PAGE_DOWN:\n> + /* We need to figure out the right commit again. */\n\nPlease delete this this comment.\n\n> + state->update_commit_ref = TRUE;\n> + return pager_request(view, request, line);\n\nCalling pager_request again.\n\n> +\n> default:\n> return pager_request(view, request, line);\n> }\n> @@ -4441,13 +4487,13 @@ static struct view_ops log_ops = {\n> \"line\",\n> { \"log\" },\n> VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER | VIEW_NO_PARENT_NAV,\n> - 0,\n> + sizeof(struct log_state),\n> log_open,\n> pager_read,\n> pager_draw,\n> log_request,\n> pager_grep,\n> - pager_select,\n> + log_select,\n> };\n>\n> struct diff_state {\n> --\n> 1.8.3.2\n>\n\n-- \nJonas Fonseca\n"},{"id":"224639","messageId":"CAFuPQ1JbX77=KdzFnPXYH5+=2+E1mxS+r8C1VsXYkpYJfMG7_A@mail.gmail.com","threadId":"34593","inReplyTo":"1375489399-11618-3-git-send-email-a.kumar@alumni.iitm.ac.in","subject":"Re: [[TIG][PATCH] 2/3] Display correct diff the context in split log view","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2013-08-06T03:50:05Z","receivedAt":"2013-08-06T03:50:05Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote:\n> In the log view, when scrolling across a commit, the diff view should\n> automatically switch to the commit whose context the cursor is on in\n> the log view. This commit changes things to catch the REQ_ENTER in the\n> log view and handle recalculation of the commit and diff display from\n> log_request, rather than delegating it to pager_request. In addition,\n> it also gets rid of unexpected upward scrolling of the log view.\n>\n> Fixes GH #155\n>\n> Signed-Off-By: Kumar Appaiah \n> ---\n> NEWS | 1 +\n> tig.c | 12 ++++++++++++\n> 2 files changed, 13 insertions(+)\n>\n> diff --git a/NEWS b/NEWS\n> index 0394407..f59e517 100644\n> --- a/NEWS\n> +++ b/NEWS\n> @@ -46,6 +46,7 @@ Bug fixes:\n> - Fix rendering glitch for branch names.\n> - Do not apply diff styling to untracked files in the stage view. (GH #153)\n> - Fix tree indentation for entries containing combining characters. (GH #170)\n> + - Introduce a more natural context-sensitive log display. (GH #155)\n>\n> tig-1.1\n> -------\n> diff --git a/tig.c b/tig.c\n> index dd4b0f4..53947b7 100644\n> --- a/tig.c\n> +++ b/tig.c\n> @@ -4478,6 +4478,18 @@ log_request(struct view *view, enum request request, struct line *line)\n> state->update_commit_ref = TRUE;\n> return pager_request(view, request, line);\n>\n> + case REQ_ENTER:\n> + /* Recalculate the correct commit for the context. */\n\nSee my dislike for this type of comments. ;)\n\n> + state->update_commit_ref = TRUE;\n> +\n> + open_view(view, REQ_VIEW_DIFF, OPEN_SPLIT);\n\nThis is called every time the user presses up/down. There should be a\ncheck that compares the VIEW(REQ_VIEW_DIFF)->ref to ref_commit.\n\n> + update_view_title(view);\n\nThis can be deleted. pager_request require this hack due to the\nautomatic scrolling (if I recall correctly).\n\n> +\n> + /* We don't want to delegate this to pager_request,\n> + since we don't want the extra scrolling of the log\n> + view. */\n\nThis explanation should IMO go into the commit message and not a\ncomment since it is somewhat confusing unless you are familiar with\nthe previous behaviour.\n\n> + return REQ_NONE;\n> +\n> default:\n> return pager_request(view, request, line);\n\nThis line can be changed to `return request;`\n\n> }\n> --\n> 1.8.3.2\n>\n\n-- \nJonas Fonseca\n"},{"id":"224640","messageId":"CAFuPQ1+5O+dBCjHYu2soAtNKiNxofAuWSVEeKBj=yu+kRGggCw@mail.gmail.com","threadId":"34593","inReplyTo":"1375489399-11618-2-git-send-email-a.kumar@alumni.iitm.ac.in","subject":"Re: [[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2013-08-06T03:54:00Z","receivedAt":"2013-08-06T03:54:00Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote:\n> diff --git a/tig.c b/tig.c\n> index 72f132a..dd4b0f4 100644\n> --- a/tig.c\n> +++ b/tig.c\n> @@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)\n> static enum request\n> log_request(struct view *view, enum request request, struct line *line)\n> {\n> + struct log_state *state = (struct log_state *) view->private;\n> +\n> switch (request) {\n> case REQ_REFRESH:\n> load_refs();\n> refresh_view(view);\n> return REQ_NONE;\n> +\n> + case REQ_MOVE_UP:\n> + case REQ_PREVIOUS:\n> + if (line->type == LINE_COMMIT && line->lineno > 1) {\n> + /* We are at a commit, and heading upward. We\n> + force log_select to find the previous\n> + commit above, from the context. */\n> + state->update_commit_ref = TRUE;\n> + }\n> + return pager_request(view, request, line);\n> +\n> + case REQ_MOVE_PAGE_UP:\n> + case REQ_MOVE_PAGE_DOWN:\n> + /* We need to figure out the right commit again. */\n> + state->update_commit_ref = TRUE;\n> + return pager_request(view, request, line);\n> +\n> default:\n> return pager_request(view, request, line);\n> }\n\nI forgot to mention there is one use case this doesn't currently\nhandle, namely jumping to a specific line using ':'. Other\nthan detecting this by tracking the current line number in log_state I\nhaven't come up with a good way to detect that a recalculation is\nrequired.\n"},{"id":"224641","messageId":"CAFuPQ1KKAM4s7h4oiUdfZ3UZXNZG7jkXYi=P38aSjJNSGayaNg@mail.gmail.com","threadId":"34593","inReplyTo":"1375489399-11618-1-git-send-email-a.kumar@alumni.iitm.ac.in","subject":"Re: [TIG][PATCH 0/3] Refactoring of the log view","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2013-08-06T04:00:19Z","receivedAt":"2013-08-06T04:00:19Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote:\n> These set of patches refactor the log view to provide a behaviour that\n> is quite similar to, say, e-mail with Mutt. The key improvements are:\n>\n> - The current commit is inferred based on the context. For example, if\n> you focus on the commit message of a particular commit, the correct\n> commit is inferred automagically.\n>\n> - Scrolling the log view when the diff is open shows the correct\n> commit on the screen, rather than have to scroll up and cross the\n> commit line to display the screen.\n\nThanks, great improvements. I am still considering whether to queue\nthem until after the next release or include them.\n\n> I have decided to revert 888611dd5d407775245d574a3dc5c01b5963a5ba,\n> since the behaviour with the updated scrolling pattern is much more\n> consistent.\n\nOK, makes sense.\n\nThe next step will be to find out how to highlight the diff stat in\nthe log view. :-D\n"},{"id":"224642","messageId":"20130806040826.GA14703@bluemoon.alumni.iitm.ac.in","threadId":"34593","inReplyTo":"CAFuPQ1KkUn5t54BXLTnYUcH_jY-SiSEJx3dDVzQ3FpswhFg0Bw@mail.gmail.com","subject":"Re: [[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view","fromName":"Kumar Appaiah","fromEmail":"a.kumar@alumni.iitm.ac.in","sentAt":"2013-08-06T04:08:26Z","receivedAt":"2013-08-06T04:08:26Z","isPatch":true,"sender":{"key":"a.kumar@alumni.iitm.ac.in","avatar":"https://gravatar.com/avatar/7ce299108bfff26792729bde4a1dd0638292f852a042d40a0a1991ff2ae58b0b?d=mp&s=160"},"body":"Dear Jonas,\n\nThanks for the patient review.\nOn Mon, Aug 05, 2013 at 11:27:44PM -0400, Jonas Fonseca wrote:\n> On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote:\n> > This commit introduces and uses the log_select function to find the\n> > correct commit in the unsplit log view. In the log view, if one\n> > scrolls down across a commit line, the current commit (as displayed in\n> > the status bar) gets updated, but not so when scrolling upward across\n> > a commit. The log_select function handles this scenario to to the\n> \n> s/to to/to do/\n\nDone.\n\n> > ``right thing''. In addition, it introduces the log_state structure as\n> > the private entry of the log view to hold a flag that decides whether\n> > to re-evaluate the current commit based on scrolling.\n> >\n> > Signed-off-by: Kumar Appaiah \n> > ---\n> > tig.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--\n> > 1 file changed, 48 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/tig.c b/tig.c\n> > index 72f132a..dd4b0f4 100644\n> > --- a/tig.c\n> > +++ b/tig.c\n> > @@ -4384,6 +4384,33 @@ pager_select(struct view *view, struct line *line)\n> > }\n> > }\n> >\n> > +struct log_state {\n> > + bool update_commit_ref;\n> > +};\n> > +\n> > +static void\n> > +log_select(struct view *view, struct line *line)\n> > +{\n> > + struct log_state *state = (struct log_state *) view->private;\n> > +\n> > + if (state->update_commit_ref && line->lineno > 1) {\n> > + /* We need to recalculate the previous commit,\n> > + since the user has likely scrolled up. */\n> \n> I'd prefer that state->update_commit_ref is given another name so it\n> won't be necessary to have these comments everywhere the field is\n> used, for example recalculate_commit_context. The comment could be\n> moved to the declaration in struct log_state to explain its use.\n\nDone.\n\n> Multi-line comments should start with '*' for each additonal line, e.g.\n> \n> /* bla bla\n> * bla bla */\n\nDone.\n\n> > + const struct line *commit_line = find_prev_line_by_type(view, line, LINE_COMMIT);\n> > +\n> > + if (commit_line)\n> > + string_copy_rev(view->ref, (char *) (commit_line->data + STRING_SIZE(\"commit \")));\n> \n> You mentioned elsewhere that this looked funny, and I guess you are\n> right. I will extract this into a utility method so you can simply\n> call: string_copy_rev_from_line(view->ref, commit_line); ...\n\nI will wait on this. The next iteration of the patch will still have\nthis problem.\n\n> > + }\n> > + if (line->type == LINE_COMMIT) {\n> > + char *text = (char *)line->data + STRING_SIZE(\"commit \");\n> > +\n> > + if (!view_has_flags(view, VIEW_NO_REF))\n> > + string_copy_rev(view->ref, text);\n> \n> ... and: string_copy_rev_from_line(view->ref, line);\n\nI understand.\n\n> > + }\n> > + string_copy_rev(ref_commit, view->ref);\n> > + state->update_commit_ref = FALSE;\n> > +}\n> > +\n> > static bool\n> > pager_open(struct view *view, enum open_flags flags)\n> > {\n> > @@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)\n> > static enum request\n> > log_request(struct view *view, enum request request, struct line *line)\n> > {\n> > + struct log_state *state = (struct log_state *) view->private;\n> \n> There's no need to cast view->private here.\n\nDone.\n\n> > +\n> > switch (request) {\n> > case REQ_REFRESH:\n> > load_refs();\n> > refresh_view(view);\n> > return REQ_NONE;\n> > +\n> > + case REQ_MOVE_UP:\n> > + case REQ_PREVIOUS:\n> > + if (line->type == LINE_COMMIT && line->lineno > 1) {\n> > + /* We are at a commit, and heading upward. We\n> > + force log_select to find the previous\n> > + commit above, from the context. */\n> \n> Please delete this comment.\n\nDone.\n\n> > + state->update_commit_ref = TRUE;\n> > + }\n> > + return pager_request(view, request, line);\n> \n> There's not really any reason to call pager_request here, since it\n> only handles REQ_ENTER.\n\nDone.\n\n> > +\n> > + case REQ_MOVE_PAGE_UP:\n> > + case REQ_MOVE_PAGE_DOWN:\n> > + /* We need to figure out the right commit again. */\n> \n> Please delete this this comment.\n\nDone.\n\n> > + state->update_commit_ref = TRUE;\n> > + return pager_request(view, request, line);\n> \n> Calling pager_request again.\n\nDone.\n\nI will send in another patch to review shortly.\n\nThanks!\n\nKumar\n-- \nKumar Appaiah\n"}]}