Download raw body.
Weird behaviour when staging binary files interactively
On Mon, Nov 04, 2024 at 05:02:08PM +1100, Mark Jamsek wrote: > I went for a minimally invasive fix rather than change the UI. I think > it makes sense to keep the same prompt and use the same "binary files > ... differ" prompt for consistency. Regress is happy but I still haven't > had time to expand it. The diff contains a basic `stage -p` test that I > used while cooking the fix, but I want to expand on this a fair bit > later tonight with plaintext and binary files in the changeset, y/n > responses, and tests for unstage and revert. > > This also fixes the diffstat insofar as it shows "1 file changed ..." > instead of 0, but it still shows "0 insertions/deletions" which we do > for binary files unless the "-a" flag to force ascii is used, in which > case the insertions/deletions are computed. I don't know if we want to > change this to always force ascii so they're unconditionally shown for > binary files; I lean to no but I'm not married to that position. Running the diff code on binaries to produce hunk info would just be a pointless waste of CPU time. One apparent problem in the diff: > @@ -5273,11 +5318,23 @@ revert_file(void *arg, unsigned char status, unsigned > > if (a->patch_cb && (status == GOT_STATUS_MODIFY || > status == GOT_STATUS_CONFLICT)) { > - int is_bad_symlink = 0; > - err = create_patched_content(&path_content, 1, &id, > - ondisk_path, dirfd, de_name, ie->path, a->repo, > + int is_bad_symlink = 0, revert_binary_file = 0; > + > + err = create_patched_content(&path_content, > + &revert_binary_file, 1, &id, ondisk_path, dirfd, > + de_name, ie->path, a->repo, > a->patch_cb, a->patch_arg); > - if (err || path_content == NULL) > + if (err != NULL) > + goto done; > + if (revert_binary_file){ > + err = install_blob(a->worktree, ondisk_path, > + ie->path, > + te ? te->mode : GOT_DEFAULT_FILE_MODE, > + got_fileindex_perms_to_st(ie), blob, > + 0, 1, 0, 0, NULL, a->repo, > + a->progress_cb, a->progress_arg); Should errors returned from install_blob() not be checked here? > + } > + if (path_content == NULL) > break; > if (te && S_ISLNK(te->mode)) { > if (unlink(path_content) == -1) {
Weird behaviour when staging binary files interactively