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

Improve behaviour of auto and periodic refresh modes #917

Merged
merged 4 commits into from
May 14, 2019

Conversation

koutcher
Copy link
Collaborator

@koutcher koutcher commented Apr 14, 2019

This PR should improve the current situation or, at least, make all the modes do something. There are still some (pre-existing) problems remaining but none of them is related in the existing issues.

As I don't use the periodic mode personally, I didn't look further but it looks to me that in this mode Tig refreshes unexpectedly 2 or 3 time at startup (hence the claim that it was working sometimes). Not the end of the world but for large repos it can be annoying.

The special handling of the stage view prevents a correct refresh of the status view in some cases.

@@ -780,10 +780,11 @@ get_input(int prompt_position, struct key *key)
while (true) {
int delay = -1;

if (opt_refresh_mode == REFRESH_MODE_PERIODIC) {
if (opt_refresh_mode != REFRESH_MODE_MANUAL) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually makes sense to me. Initially, the watch feature was lazy to avoid triggering a lot of unnecessary work. Clearly it was too lazy.

src/watch.c Outdated
!index_diff(&diff, opt_show_untracked, false))
if (event == WATCH_EVENT_SWITCH_VIEW ||
!(check_file_mtime(&handler->last_modified, "%s/index", repo.git_dir) ||
index_diff(&diff, opt_show_untracked, false)))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 better captures the boolean logic here.

Copy link
Collaborator Author

@koutcher koutcher May 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part was rewritten to make sure we can detect several successive modification.

@koutcher

This comment has been minimized.

@koutcher koutcher force-pushed the gh-794-refresh-mode branch 11 times, most recently from 4268489 to 284a914 Compare May 4, 2019 18:47
src/stage.c Outdated
@@ -459,8 +461,10 @@ stage_request(struct view *view, enum request request, struct line *line)

/* Check whether the staged entry still exists, and close the
* stage view if it doesn't. */
if (view->parent && !stage_exists(view, &stage_status, stage_line_type))
if (view->parent && !stage_exists(view, &stage_status, stage_line_type)) {
stage_line_type = 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point to clean up the state before closing

@koutcher koutcher force-pushed the gh-794-refresh-mode branch 4 times, most recently from fbf9ee8 to e670064 Compare May 9, 2019 17:17
@koutcher koutcher requested a review from jonas May 9, 2019 20:20
@koutcher koutcher force-pushed the gh-794-refresh-mode branch from b5d05f6 to 0f00a6b Compare May 10, 2019 16:19
@jonas
Copy link
Owner

jonas commented May 10, 2019

Still looks good to me.

@koutcher koutcher force-pushed the gh-794-refresh-mode branch 6 times, most recently from f409533 to 517d46e Compare May 12, 2019 21:29
@koutcher koutcher force-pushed the gh-794-refresh-mode branch from 517d46e to 78012db Compare May 14, 2019 17:15
@koutcher koutcher merged commit 89667b4 into jonas:master May 14, 2019
koutcher added a commit that referenced this pull request Mar 2, 2020
The extra load_refs() introduced by #591 to fix #430 is no longer needed
after #917 and causes the loop.

Fixes #991
@koutcher koutcher deleted the gh-794-refresh-mode branch February 6, 2025 18:18
@koutcher koutcher restored the gh-794-refresh-mode branch February 6, 2025 18:36
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