Duy Nguyen <pclo...@gmail.com> writes: >> The internal implementation detail of am_abort() is leaking out >> here, by saying "rerere-clear" is the only special thing other than >> recovering the HEAD and working tree state when abort happens. It >> makes readers wonder if am_rerere_clear() should become part of >> am_destroy(). I dunno. > > I think the original design is am_destroy takes care of > $GIT_DIR/rebase-apply and nothing else. --abort has to clean up things > outside (index, HEAD, rerere) while a successful operation should not > leave anything else to clean up (except rebase-apply dir).
Yes, and that is why I think the code would have been nicer to understand if the update to add 'reset' had turned the existing am_abort() into a helper that is one level higher in the abstraction (perhaps even by renaming the function) that the caller can tell which part to clear (i.e. e.g. am_finish(&state, AM_CLEAR_ALL) vs am_finish(&state, AM_CLEAR_STEP_ONLY)). Stepping back even further, perhaps the call made to am_destroy() in the normal exit case at the end of am_run() could have been using the same helper.