From: "Ævar Arnfjörð Bjarmason" <email@example.com> To: Firmin Martin <firstname.lastname@example.org> Cc: email@example.com, Junio C Hamano <firstname.lastname@example.org>, Jeff King <email@example.com>, Johannes Schindelin <firstname.lastname@example.org>, Erik Faye-Lund <email@example.com>, Denton Liu <firstname.lastname@example.org> Subject: Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite Date: Mon, 10 May 2021 14:02:19 +0200 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Thu, May 06 2021, Firmin Martin wrote: > BACKGROUND > ========== > > Currently, git-format-patch, along with the option --cover-letter, > unconditionally overwrites a cover letter with the same name (if > present). Although this is a desired behaviour for patches which are > auto-generated from Git commits log, it might not be the case for a > cover letter whose the content is meticulously written manually. > > Particulary, this behaviour could be awkward in the following > hypothetical situations: > > * The user can easily erase a cover letter coming from prior versions or > another patch series by reusing an old command line (e.g. autocompleted > from the shell history). > > * Assuming that the user is writing a cover letter and realizes that > small changes should be made. They make the change, amend and > format-patch again to regenerate patches. If it happens that they use > the same command again (e.g. with --cover-letter), the cover letter > being written is gone. > > This patch series addresses this kind of issue by asking confirmation > from the user whenever a cover letter or a patch is subject to be > overwritten. I like the goal here, I'm another person with ad-hoc tooling around format-patch to manage / avoid this particular scenario and related issues (e.g. I have a wrapper to rm -rf the output directory & re-crete it, in case I want to rebase but use the same -vN version). I wonder if you've considered some ways to automatically and more gently detect these cases, such as: 1. When we generate the patch, set the mtime manually to the time in the commit object. When clobbering a file see if they correspond. If mtime != expected => *boom* and ask for confirmation. 2. We already include a blurb like "184.108.40.2068.g7ac6e98bb53" (the git version) if you have format.signature set. How about making that include a short hash of the preceding lines. If it doesn't match we can ask, but if it does we clobber. This has the added benefit that other could script their MUAs to highlight manually edited patches. 3. Ditto #2 but generate the new file in a tempfile, diff them, if they're different complain. This also opens the door to make this neatly integrate with git-diff's -I option, so you could specify regexes to ignore. By default we could ignore lines that change known headers we expect to change. 4. The format we output would need parsing, but it's not that hard. I.e. we expect something like: [...] Subject: [PATCH 0/1] *** SUBJECT HERE *** MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *** BLURB HERE *** Ævar Arnfjörð Bjarmason (1): [...] And similarly for patches we could narrow things to look between the "---" and an expected diffstat. So we could complain just about changes there. But maybe that's a fool's errand, e.g. it would be hard to catch manually commented-on range-diffs without implementing a full parser for that format... None of these suggestions should be read as making perfect the enemy of the good. I *do* rely on the behavior of the setting you're introducing and "breaking", but I think the user-base of advanced format-patch users is small enough that we could just configure things to "never" and move on, but accidentally losing data (as happens now) is a worse default...
prev parent reply other threads:[~2021-05-10 13:40 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-06 16:50 Firmin Martin 2021-05-06 16:50 ` [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe Firmin Martin 2021-05-06 23:37 ` Junio C Hamano 2021-05-07 4:54 ` Jeff King 2021-05-07 5:25 ` Junio C Hamano 2021-05-10 4:18 ` Firmin Martin 2021-05-10 21:32 ` Jeff King 2021-05-11 3:38 ` Junio C Hamano 2021-05-11 6:10 ` Jeff King 2021-05-11 6:17 ` Junio C Hamano 2021-05-11 6:37 ` Jeff King 2021-05-06 16:50 ` [PATCH v1 2/8] format-patch: confirmation whenever patches exist Firmin Martin 2021-05-06 23:48 ` Junio C Hamano 2021-05-10 3:30 ` Firmin Martin 2021-05-10 7:32 ` Junio C Hamano 2021-05-11 3:17 ` Firmin Martin 2021-05-06 16:50 ` [PATCH v1 3/8] format-patch: add config option confirmOverwrite Firmin Martin 2021-05-06 16:50 ` [PATCH v1 4/8] format-patch: add the option --confirm-overwrite Firmin Martin 2021-05-06 16:50 ` [PATCH v1 5/8] t4014: test patches overwrite confirmation Firmin Martin 2021-05-06 16:51 ` [PATCH v1 6/8] t4014: fix tests overwriting cover letter in silent Firmin Martin 2021-05-06 16:51 ` [PATCH v1 7/8] doc/format-patch: describe --confirm-overwrite Firmin Martin 2021-05-07 3:32 ` Bagas Sanjaya 2021-05-10 4:22 ` Firmin Martin 2021-05-06 16:51 ` [PATCH v1 8/8] config/format: describe format.confirmOverwrite Firmin Martin 2021-05-06 22:33 ` [PATCH v1 0/8] format-patch: introduce --confirm-overwrite Junio C Hamano 2021-05-11 0:18 ` Firmin Martin 2021-05-07 1:46 ` Felipe Contreras 2021-05-07 8:55 ` Denton Liu 2021-05-11 1:09 ` Firmin Martin 2021-05-11 5:12 ` Felipe Contreras 2021-05-11 5:03 ` Felipe Contreras 2021-05-07 14:02 ` Sergey Organov 2021-05-11 0:46 ` Firmin Martin 2021-05-10 12:02 ` Ævar Arnfjörð Bjarmason [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).