From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.devel Subject: Re: master 4803fba487 1/2: 'C-x v v' on a diff buffer commits it as a patch (bug#52349) Date: Tue, 30 Aug 2022 15:35:31 +0300 Message-ID: References: <166171593185.16640.41619657947456727@vcs2.savannah.gnu.org> <20220828194533.23A6BC00889@vcs2.savannah.gnu.org> <87r10znm0y.fsf@gnus.org> <83fshfvvyn.fsf@gnu.org> <83bks3vtf5.fsf@gnu.org> <83tu5uug68.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40145"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Cc: larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Aug 30 14:41:26 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oT0YY-000AKD-MN for ged-emacs-devel@m.gmane-mx.org; Tue, 30 Aug 2022 14:41:26 +0200 Original-Received: from localhost ([::1]:57636 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oT0YX-0005SX-Q9 for ged-emacs-devel@m.gmane-mx.org; Tue, 30 Aug 2022 08:41:25 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:42480) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oT0TD-0000QX-Ak for emacs-devel@gnu.org; Tue, 30 Aug 2022 08:35:55 -0400 Original-Received: from mail-wr1-x42e.google.com ([2a00:1450:4864:20::42e]:41618) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oT0TA-0003ZW-QJ; Tue, 30 Aug 2022 08:35:54 -0400 Original-Received: by mail-wr1-x42e.google.com with SMTP id v16so11299111wrm.8; Tue, 30 Aug 2022 05:35:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:from:to:cc; bh=cKTnSams7LKIAIR5m/BZFAoi89XIL7Q5MBNGWZOEuZ0=; b=hWYi5c3E2eor/2Bwa83q8vg6KJZw621oX80OzjEUjZVwzWHH5oHjR8aXklsSxCbAQ2 1sLfdgYjWAbG6yJOs3KhTlR/7TSgmwrGdgbwiR4tdO1cEDS5KWqigmThvFjhTIXhEE2k 0XoutrnTQkCJmsJs6b+5OxvlKWBqswG2ppkIACw69vtNc/3ohyg9zzDkd5fbfiuqY8Fi 1eoW1yD1VkAX/18CB1AwKYO/k70gUBezo0TXZoYoeQtM2JE4GWrSxnUW3A1Wm/2Xu+om dNdGSbfgQu/HIjoiBqiQrl0g6QJQjsI/TbleJD527pgjH/PbLWaCHdszrnJGwvxaVimj Tb+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:x-gm-message-state:from:to:cc; bh=cKTnSams7LKIAIR5m/BZFAoi89XIL7Q5MBNGWZOEuZ0=; b=nJGdL95ZUyAmWy/kGT5OukrSX8JGkmixb9tfQuJgTqvgG+8w0KbsGD0ennaSs4BoXl rUL0cvc8N4bgoAPkqJD0aAHvJkE6kkUCixCZgPL2E97njDs0d93IAGmsT2nPFkMLQTLQ GYYAbt5sDy9sbPQDvde5TVPibZeW6LSgcZ3kcILIOtRwku6XOrSoHyYv90vnQNqtL44/ /dMuo3VBJPwcTmTV1HgXkZIICyoNSW6iOhijKL0XD0ZTlRlBzUN+Dt6bCbG4+IpFdayI +QU4UjfRMCMGEr8o0Kr5c30lOP5KLzHrF3AudAwj3zLyLaXj6CWLN1B5oNhADYk/1mU4 Wrwg== X-Gm-Message-State: ACgBeo2+Zs7jnP0FAXbFv5wYyAwtlaK9atI3K7+UlOW2YLZJ82KrT1ZE X7E7abXM1lMD6C+fevduwZz6xDRZy/o= X-Google-Smtp-Source: AA6agR6L3HbrlKW0cI208SsH2IfxqzE8CbytSWOfaX9jnb80/jUh0uvyZ/wWrx62iF3hDUCjinbe1g== X-Received: by 2002:adf:f14b:0:b0:225:8b5a:cc28 with SMTP id y11-20020adff14b000000b002258b5acc28mr8521415wro.265.1661862933136; Tue, 30 Aug 2022 05:35:33 -0700 (PDT) Original-Received: from [192.168.0.215] (buscust41-118.static.cytanet.com.cy. [212.31.107.118]) by smtp.googlemail.com with ESMTPSA id r8-20020a5d6948000000b00226a5187528sm9394394wrw.48.2022.08.30.05.35.31 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 30 Aug 2022 05:35:32 -0700 (PDT) Content-Language: en-US In-Reply-To: <83tu5uug68.fsf@gnu.org> Received-SPF: pass client-ip=2a00:1450:4864:20::42e; envelope-from=raaahh@gmail.com; helo=mail-wr1-x42e.google.com X-Spam_score_int: -13 X-Spam_score: -1.4 X-Spam_bar: - X-Spam_report: (-1.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FORGED_FROMDOMAIN=0.25, FREEMAIL_FROM=0.001, GAPPY_SUBJECT=0.1, HEADER_FROM_DIFFERENT_DOMAINS=0.25, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:294338 Archived-At: On 30.08.2022 14:25, Eli Zaretskii wrote: >> Date: Mon, 29 Aug 2022 23:36:32 +0300 >> Cc: larsi@gnus.org, juri@jurta.org, emacs-devel@gnu.org >> From: Dmitry Gutov >> >>> The implementation in vc-git.el does this, AFAICT: >>> >>> . creates a temporary file >>> . inserts the patch into the temporary file >> >> Note that "the patch" will usually be a subset of the current 'git diff' >> output. But not always (the patch can be additionally modified by hand). > > I understand. But how is this significant? Doesn't "git apply" > require a valid patch as well? It's significant in that the starting conditions is that the files are already modified compared to the repository head, and the end goal is to be able to pick only some of those changes for commit. To partition a complex change into several commits, usually. Or to simply keep unfinished changes separate without resorting to 'git stash'. (**) >>> . calls "git apply --cached" on the patch in the temporary file >>> . deletes the temporary file >>> >>> This doesn't seem to be very different from invoking Patch on the >>> diffs. (And using shell-command-on-region avoids the need to >>> explicitly create a temporary file.) >> >> 'patch < file' edits the working directory. 'git apply --cached' edits >> the index, keeping the files themselves unchanged. > > I'm not sure why is this significant. Doesn't committing the patch > from the index eventually update the files on disk as well? Nope. > And if > not, how is this feature useful? you get a repository where the > contents of the .git/ directory doesn't match the worktree, right? > When is this needed or useful? See above. The two sentences near near the (**) sign. You can also commit a patch with changes that aren't present in the files and then 'vc-revert' in them. But doing that kind of thing wasn't particularly difficult even before this feature landed IMHO. >>>> Personally I hope we discover some popular extension to Mercurial which >>>> we'll be able to use in the same way as we do Git's index area here. And >>>> then say job well done and keep the less-popular and outdated backends >>>> unsupported. >>> >>> The index thing being the problem because Git needs to have the >>> changes in the index before you can commit? Or is there any other >>> reason? >> >> Index is a solution, not the problem. The problem is other changes being >> present on disk. > > For other changes present on disk, we cam: > > . use the equivalent of the "stash" command, where the VCS supports it > . otherwise, commit specific files by name, where the VCS supports that > . otherwise signal an error, asking the user to clean up the tree Like I said, I'm worried about the resulting differences in behavior. That will make it much harder for us to document the new feature properly, and the users who work with different backends from time to time are likely to stumble over those differences. So far using /tmp and copying seems like a better approach. >> I suppose we could implement a scaled-down version of this feature as a >> fallback (the one that ensures that there are no existing changes in >> affected files), but I fear the difference between the backends would >> trip up the users. > > AFAICT, Mercurial, Bazaar, RCS, CVS, and SVN all support committing an > explicit list of files, so dirty working tree shouldn't be a problem > for them. The first two also have a "stash"-like functionality. Not dirty working tree. "Dirty" specific files which we want to commit certain changes in. > But even if we initially only support this in otherwise clean trees, > it will be a huge advantage vs the current situation, where this > feature is simply not available. Will the advantage be significant? I really have to ask, because IME applying an external patch and them committing the files as a whole never felt particularly painful before (aside from resolving any conflicts or corrupted whitespace, which the new feature doesn't help with). And 'diff-mode' has had the 'diff-apply-hunk' command for ages -- I use it often. OTOH, the lack of support for workflow corresponding to 'git add -p' has been complained about frequently, it's one of the reasons people use Magit, and I felt it as well many times. To improve your workflow, though, might it help to add a new command 'diff-apply-buffer'?