Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP improved context-sensitive view-blob #683

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rolandwalker
Copy link
Contributor

I would like to improve the sensitivity of some tig actions to their context.

This is expressed here as three TODO tests which call view-blob from the diff view.

Tig already adopts the model of context-sensitive action here, but it depends on the cursor directly touching a line which it can associate to a blob. When that is not true, tig opens the last blob it can remember, or opens the file finder.

Neither of those is likely to match user expectations. But when viewing a diff, the cursor is always either on a blob-associated line, or before some blob-associated line. So I propose that the simplest intuitive context-sensitive view-blob is

  • view this line's blob, or failing that
  • scan forward by lines and view the next associated blob

@rolandwalker rolandwalker force-pushed the view-blob-context-todo branch from 199a6e9 to 4d8d2f3 Compare July 26, 2017 23:58
@jonas
Copy link
Owner

jonas commented Jul 30, 2017

This makes sense and sounds like it will bring view-blob more in line with how the edit action works. However it is a bit hard to understand the behavior from only looking at the todo tests.

@rolandwalker
Copy link
Contributor Author

I got as far as the following, which passes the included todos, but fails many others.

diff --git a/src/diff.c b/src/diff.c
index 583f1e0e0..29e002691 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -624,16 +624,16 @@ diff_get_pathname(struct view *view, struct line *line)
 	int i;

 	header = find_prev_line_by_type(view, line, LINE_DIFF_HEADER);
-	if (!header)
-		return NULL;

-	for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
-		dst = strstr(box_text(header), prefixes[i]);
-		if (dst)
-			return dst + strlen(prefixes[i]);
-	}
+	if (header)
+		for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
+			dst = strstr(box_text(header), prefixes[i]);
+			if (dst)
+				return dst + strlen(prefixes[i]);
+		}
+
+	header = find_next_line_by_type(view, header ? header : line, LINE_DIFF_ADD_FILE);

-	header = find_next_line_by_type(view, header, LINE_DIFF_ADD_FILE);
 	if (!header)
 		return NULL;

@rolandwalker rolandwalker force-pushed the view-blob-context-todo branch 3 times, most recently from 2c83709 to c57521f Compare August 2, 2017 12:06
@rolandwalker rolandwalker changed the title RFC tests defining improved context-sensitive view-blob WIP improved context-sensitive view-blob Aug 2, 2017
@rolandwalker rolandwalker force-pushed the view-blob-context-todo branch 3 times, most recently from ffa5045 to 10ffa9f Compare August 7, 2017 15:01
@rolandwalker rolandwalker force-pushed the view-blob-context-todo branch 3 times, most recently from 958952f to 2bbf1fa Compare August 17, 2017 15:19
@rolandwalker rolandwalker force-pushed the view-blob-context-todo branch from 2bbf1fa to 8a695c0 Compare May 21, 2018 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants