Restore the animation origin and its viewport row, and retain the mapped time-travel destination through refreshes. When a refresh no longer contains the selected object ID, match the displayed entry number within its visual base, then keep the nearest prior row before falling back to the top. Schedule refreshes when lazy finalization changes refs without producing a checkout notice.
Unified · Split
| @@ -524,7 +524,7 @@ pub(crate) struct App { · 523 unmodified lines | ||
| 524 | 524 | compressed_anchor: Option<ObjectId>, |
| 525 | 525 | compressed_segment: Option<ObjectId>, |
| 526 | 526 | compressed_expanded: HashSet<ObjectId>, |
| 527 | − time_travel_animation: Option<ObjectId>, | |
| 527 | + time_travel_animation: Option<(ObjectId, usize)>, | |
| 528 | 528 | attributions: Vec<Attribution>, |
| 529 | 529 | #[cfg(test)] |
| 530 | 530 | test_lanes: Vec<String>, |
| @@ -2339,6 +2339,12 @@ impl App { · 1808 unmodified lines | ||
| 2339 | 2339 | return; |
| 2340 | 2340 | } |
| 2341 | 2341 | let select_top = std::mem::take(&mut self.select_top_after_refresh); |
| 2342 | + let previous_selection = if select_top { None } else { self.selected }; | |
| 2343 | + let previous_visual_position = previous_selection.and_then(|index| { | |
| 2344 | + let count = self.visual_count(index)?; | |
| 2345 | + let base = self.rows.get(index.checked_add(count)?)?.id; | |
| 2346 | + Some((base, count, index)) | |
| 2347 | + }); | |
| 2342 | 2348 | let segment = (!select_top && self.selection_after_refresh.is_none()) |
| 2343 | 2349 | .then_some(self.compressed_segment) |
| 2344 | 2350 | .flatten(); |
| @@ -2395,8 +2401,33 @@ impl App { · 50 unmodified lines | ||
| 2395 | 2401 | self.graph = Some(graph); |
| 2396 | 2402 | self.lane_time = Some(lane_time); |
| 2397 | 2403 | self.update_worktree_head_descendants(); |
| 2404 | + let visual_counts = &self | |
| 2405 | + .graph | |
| 2406 | + .as_ref() | |
| 2407 | + .expect("the completed graph was just stored") | |
| 2408 | + .visual_counts; | |
| 2409 | + let visual_selection = previous_visual_position.and_then(|(base, count, previous)| { | |
| 2410 | + self.rows | |
| 2411 | + .iter() | |
| 2412 | + .enumerate() | |
| 2413 | + .filter(|(index, _)| { | |
| 2414 | + visual_counts.get(*index) == Some(&count) | |
| 2415 | + && index | |
| 2416 | + .checked_add(count) | |
| 2417 | + .and_then(|base_index| self.rows.get(base_index)) | |
| 2418 | + .is_some_and(|row| row.id == base) | |
| 2419 | + }) | |
| 2420 | + .min_by_key(|(index, _)| index.abs_diff(previous)) | |
| 2421 | + .map(|(index, _)| index) | |
| 2422 | + }); | |
| 2398 | 2423 | self.selected = selected |
| 2399 | 2424 | .and_then(|id| self.rows.iter().position(|row| row.id == id)) |
| 2425 | + .or(visual_selection) | |
| 2426 | + .or_else(|| { | |
| 2427 | + previous_selection | |
| 2428 | + .filter(|_| !self.rows.is_empty()) | |
| 2429 | + .map(|index| index.min(self.rows.len() - 1)) | |
| 2430 | + }) | |
| 2400 | 2431 | .or_else(|| (!self.rows.is_empty()).then_some(0)); |
| 2401 | 2432 | self.compressed_segment = segment; |
| 2402 | 2433 | if self.compressed_segment.is_some() { |
| @@ -3094,13 +3125,13 @@ impl App { · 691 unmodified lines | ||
| 3094 | 3125 | } |
| 3095 | 3126 | |
| 3096 | 3127 | pub(crate) fn begin_time_travel_animation(&mut self) { |
| 3097 | − let viewport_row = if self.alignment == Alignment::Compressed { | |
| 3098 | − self.selected_history_index() | |
| 3099 | − .map(|selected| selected.saturating_sub(self.offset)) | |
| 3100 | − } else { | |
| 3101 | − None | |
| 3102 | − }; | |
| 3103 | − self.time_travel_animation = self.selected.and_then(|index| self.rows.get(index)).map(|row| row.id); | |
| 3128 | + let viewport_row = self | |
| 3129 | + .selected_history_index() | |
| 3130 | + .map(|selected| selected.saturating_sub(self.offset)); | |
| 3131 | + self.time_travel_animation = self | |
| 3132 | + .selected | |
| 3133 | + .and_then(|index| self.rows.get(index)) | |
| 3134 | + .map(|row| (row.id, viewport_row.unwrap_or_default())); | |
| 3104 | 3135 | if let (Some(selected), Some(viewport_row)) = (self.selected_history_index(), viewport_row) { |
| 3105 | 3136 | self.offset = selected.saturating_sub(viewport_row); |
| 3106 | 3137 | } |
| @@ -3108,25 +3139,23 @@ impl App { · 1 unmodified line | ||
| 3108 | 3139 | } |
| 3109 | 3140 | |
| 3110 | 3141 | pub(crate) fn finish_time_travel_animation(&mut self) { |
| 3111 | − let viewport_row = if self.alignment == Alignment::Compressed { | |
| 3112 | − self.selected_history_index() | |
| 3113 | − .map(|selected| selected.saturating_sub(self.offset)) | |
| 3114 | − } else { | |
| 3115 | − None | |
| 3116 | − }; | |
| 3117 | − if self.time_travel_animation.take().is_none() || self.alignment != Alignment::Compressed { | |
| 3142 | + let Some((origin, viewport_row)) = self.time_travel_animation else { | |
| 3118 | 3143 | return; |
| 3144 | + }; | |
| 3145 | + self.select_commit(origin); | |
| 3146 | + self.time_travel_animation = None; | |
| 3147 | + if self.alignment == Alignment::Compressed { | |
| 3148 | + self.compressed_anchor = self.selected.and_then(|index| self.rows.get(index)).map(|row| row.id); | |
| 3149 | + self.rebuild_compressed_history(); | |
| 3119 | 3150 | } |
| 3120 | − self.compressed_anchor = self.selected.and_then(|index| self.rows.get(index)).map(|row| row.id); | |
| 3121 | − self.rebuild_compressed_history(); | |
| 3122 | − if let (Some(selected), Some(viewport_row)) = (self.selected_history_index(), viewport_row) { | |
| 3151 | + if let Some(selected) = self.selected_history_index() { | |
| 3123 | 3152 | self.offset = selected.saturating_sub(viewport_row); |
| 3124 | 3153 | } |
| 3125 | 3154 | self.ensure_visible(); |
| 3126 | 3155 | } |
| 3127 | 3156 | |
| 3128 | 3157 | pub(crate) fn time_travel_animation_origin(&self) -> Option<ObjectId> { |
| 3129 | − self.time_travel_animation | |
| 3158 | + self.time_travel_animation.map(|(origin, _)| origin) | |
| 3130 | 3159 | } |
| 3131 | 3160 | |
| 3132 | 3161 | pub(crate) fn select_commit_after_refresh(&mut self, id: ObjectId) { |
| @@ -4245,6 +4274,60 @@ mod tests { · 1112 unmodified lines | ||
| 4245 | 4274 | ); |
| 4246 | 4275 | } |
| 4247 | 4276 | |
| 4277 | + #[test] | |
| 4278 | + fn refresh_restores_a_rewritten_selection_by_its_visual_position() { | |
| 4279 | + let mut app = App::new(3); | |
| 4280 | + app.extend_commits(vec![ | |
| 4281 | + row_with_parents(6, &[5]), | |
| 4282 | + row_with_parents(5, &[4]), | |
| 4283 | + row_with_parents(4, &[3]), | |
| 4284 | + row_with_parents(3, &[2]), | |
| 4285 | + row_with_parents(2, &[1]), | |
| 4286 | + row(1), | |
| 4287 | + ]); | |
| 4288 | + complete(&mut app); | |
| 4289 | + app.select_commit(id(4)); | |
| 4290 | + assert_eq!(app.visual_count(app.selected.expect("commit 4 is selected")), Some(3)); | |
| 4291 | + assert_eq!(app.offset, 0, "the original selection is at the bottom of the viewport"); | |
| 4292 | + | |
| 4293 | + let rows = app.store_commits( | |
| 4294 | + vec![ | |
| 4295 | + numbered_row(17, Some(16)), | |
| 4296 | + numbered_row(16, Some(15)), | |
| 4297 | + numbered_row(15, Some(14)), | |
| 4298 | + numbered_row(14, Some(13)), | |
| 4299 | + numbered_row(13, Some(2)), | |
| 4300 | + numbered_row(2, Some(1)), | |
| 4301 | + numbered_row(1, None), | |
| 4302 | + ] | |
| 4303 | + .into(), | |
| 4304 | + ); | |
| 4305 | + app.state = State::Computing; | |
| 4306 | + let (rows, graph, time) = compute_lanes(rows); | |
| 4307 | + app.finish_lane_computation(rows, graph, time); | |
| 4308 | + | |
| 4309 | + assert_eq!( | |
| 4310 | + app.selected.map(|index| app.rows[index].id), | |
| 4311 | + Some(id(14)), | |
| 4312 | + "the same base and displayed entry number identify the rewritten commit" | |
| 4313 | + ); | |
| 4314 | + assert_eq!( | |
| 4315 | + app.offset, 1, | |
| 4316 | + "the restored selection stays at the bottom of the viewport" | |
| 4317 | + ); | |
| 4318 | + | |
| 4319 | + let rows = app.store_commits(vec![numbered_row(9, Some(8)), numbered_row(8, None)].into()); | |
| 4320 | + app.state = State::Computing; | |
| 4321 | + let (rows, graph, time) = compute_lanes(rows); | |
| 4322 | + app.finish_lane_computation(rows, graph, time); | |
| 4323 | + | |
| 4324 | + assert_eq!( | |
| 4325 | + app.selected.map(|index| app.rows[index].id), | |
| 4326 | + Some(id(8)), | |
| 4327 | + "a missing visual position clamps the previous row before falling back to the top" | |
| 4328 | + ); | |
| 4329 | + } | |
| 4330 | + | |
| 4248 | 4331 | #[test] |
| 4249 | 4332 | fn selected_head_follows_an_external_rewrite() { |
| 4250 | 4333 | let mut app = App::new(10); |
| @@ -5360,6 +5443,49 @@ mod tests { · 1109 unmodified lines | ||
| 5360 | 5443 | ); |
| 5361 | 5444 | } |
| 5362 | 5445 | |
| 5446 | + #[test] | |
| 5447 | + fn time_travel_animation_returns_to_its_origin() { | |
| 5448 | + let mut app = App::new(4); | |
| 5449 | + app.extend_commits( | |
| 5450 | + (1u16..=10) | |
| 5451 | + .rev() | |
| 5452 | + .map(|commit| numbered_row(commit, (commit > 1).then_some(commit - 1))) | |
| 5453 | + .collect::<Vec<_>>(), | |
| 5454 | + ); | |
| 5455 | + complete(&mut app); | |
| 5456 | + app.select_commit(id(6)); | |
| 5457 | + assert_eq!((app.selected, app.offset), (Some(4), 1)); | |
| 5458 | + | |
| 5459 | + app.begin_time_travel_animation(); | |
| 5460 | + app.select_commit_for_time_travel(id(10)); | |
| 5461 | + app.finish_time_travel_animation(); | |
| 5462 | + | |
| 5463 | + assert_eq!( | |
| 5464 | + (app.selected.map(|index| app.rows[index].id), app.offset), | |
| 5465 | + (Some(id(6)), 1), | |
| 5466 | + "the animated cursor returns to the requested entry and viewport row" | |
| 5467 | + ); | |
| 5468 | + | |
| 5469 | + app.alignment = Alignment::None; | |
| 5470 | + app.update(Action::ToggleAlign); | |
| 5471 | + let viewport_row = app | |
| 5472 | + .selected_history_index() | |
| 5473 | + .expect("the origin is displayed in compressed history") | |
| 5474 | + .saturating_sub(app.offset); | |
| 5475 | + app.begin_time_travel_animation(); | |
| 5476 | + app.select_commit_for_time_travel(id(10)); | |
| 5477 | + app.finish_time_travel_animation(); | |
| 5478 | + | |
| 5479 | + assert_eq!(app.selected.map(|index| app.rows[index].id), Some(id(6))); | |
| 5480 | + assert_eq!( | |
| 5481 | + app.selected_history_index() | |
| 5482 | + .expect("the origin remains displayed") | |
| 5483 | + .saturating_sub(app.offset), | |
| 5484 | + viewport_row, | |
| 5485 | + "compressed history restores the origin to its former viewport row" | |
| 5486 | + ); | |
| 5487 | + } | |
| 5488 | + | |
| 5363 | 5489 | #[test] |
| 5364 | 5490 | fn time_travel_animation_temporarily_expands_compressed_history() { |
| 5365 | 5491 | let mut app = App::new(4); |
| @@ -3266,6 +3266,7 @@ fn event_loop( · 3265 unmodified lines | ||
| 3266 | 3266 | match result { |
| 3267 | 3267 | Ok(edit::time_travel::Perform::Complete { |
| 3268 | 3268 | notice: Some(notice), |
| 3269 | + selected, | |
| 3269 | 3270 | ref_changes, |
| 3270 | 3271 | .. |
| 3271 | 3272 | }) => { |
| @@ -3278,20 +3279,18 @@ fn event_loop( · 6 unmodified lines | ||
| 3278 | 3279 | &ref_changes, |
| 3279 | 3280 | notice, |
| 3280 | 3281 | ); |
| 3281 | − if let Ok(head) = open_repository(&repository_path, repository_is_bare, false) | |
| 3282 | − .and_then(|repo| Ok(repo.head_id()?.detach())) | |
| 3283 | − { | |
| 3284 | − app.select_commit_after_refresh(head); | |
| 3285 | − } | |
| 3282 | + app.select_commit_after_refresh(selected); | |
| 3286 | 3283 | invalidate_worktree_changes(&mut worktree_changes); |
| 3287 | 3284 | refresh_pending = true; |
| 3288 | 3285 | } |
| 3289 | 3286 | Ok(edit::time_travel::Perform::Complete { |
| 3290 | 3287 | notice: None, |
| 3288 | + selected, | |
| 3291 | 3289 | ref_changes, |
| 3292 | 3290 | .. |
| 3293 | 3291 | }) => { |
| 3294 | 3292 | if !ref_changes.is_empty() { |
| 3293 | + app.select_commit_after_refresh(selected); | |
| 3295 | 3294 | leave_recorded_success( |
| 3296 | 3295 | &mut app, |
| 3297 | 3296 | &repository_path, |
| @@ -3300,6 +3299,8 @@ fn event_loop( · 2 unmodified lines | ||
| 3300 | 3299 | &ref_changes, |
| 3301 | 3300 | "time-travelled", |
| 3302 | 3301 | ); |
| 3302 | + invalidate_worktree_changes(&mut worktree_changes); | |
| 3303 | + refresh_pending = true; | |
| 3303 | 3304 | } |
| 3304 | 3305 | } |
| 3305 | 3306 | Ok(edit::time_travel::Perform::Conflict(conflict)) => { |