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.bugs Subject: bug#64055: [WIP Patch] Enable editing commit messages - vc-git-modify-change-comment Date: Sat, 17 Jun 2023 05:40:09 +0300 Message-ID: <0e2fda3f-7952-66d3-1b13-19ee31dbad7a@gutov.dev> References: 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="24546"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 To: Morgan Smith , 64055@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jun 17 04:41:25 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1qALsR-0006DQ-Rt for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 17 Jun 2023 04:41:24 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qALs9-00036W-9s; Fri, 16 Jun 2023 22:41:05 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qALs7-000360-Go for bug-gnu-emacs@gnu.org; Fri, 16 Jun 2023 22:41:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qALs7-0005W0-7w for bug-gnu-emacs@gnu.org; Fri, 16 Jun 2023 22:41:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qALs6-0004dJ-Bz for bug-gnu-emacs@gnu.org; Fri, 16 Jun 2023 22:41:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 17 Jun 2023 02:41:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 64055 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 64055-submit@debbugs.gnu.org id=B64055.168696962317756 (code B ref 64055); Sat, 17 Jun 2023 02:41:02 +0000 Original-Received: (at 64055) by debbugs.gnu.org; 17 Jun 2023 02:40:23 +0000 Original-Received: from localhost ([127.0.0.1]:50646 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qALrT-0004cJ-5F for submit@debbugs.gnu.org; Fri, 16 Jun 2023 22:40:23 -0400 Original-Received: from wout3-smtp.messagingengine.com ([64.147.123.19]:59219) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qALrQ-0004c3-Oo for 64055@debbugs.gnu.org; Fri, 16 Jun 2023 22:40:22 -0400 Original-Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.west.internal (Postfix) with ESMTP id D45AD3200923; Fri, 16 Jun 2023 22:40:12 -0400 (EDT) Original-Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Fri, 16 Jun 2023 22:40:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm3; t= 1686969612; x=1687056012; bh=6+VgdQdJvTLvXo3f2AEMea4YDAQZnjXUodd pI0ZjK1w=; b=0nNygG8OV/VqrcxbxtszltKC5CV/EKihQP1inaLUwcyLBYX3Oui SazJnT9fIGYfndTDGfPeqUzJZ9M5FeNYZpsbPFX59eRChSq1TP9zFTCJ8QZAmP4o JnxUAPtJsmxR7XwPJFNevw/OFHuek6dtDgyn2ILaHtsk9ncas7T7WgnLYghqfBh9 puhvDR9SyfpPJtS4Y17giLV9bxP437Aw92T6q7rCgboXzGF87mT3UfiatODCaji5 VDlJTRcuXBtVuJ93+Vsii1y29FgCHu/i/6vNfDeBE0VyIh2DYuunb25pMTqtFysj BFBCbjgHv8llHC5Pjw/xAD/bkA5nGqLK2mQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1686969612; x= 1687056012; bh=6+VgdQdJvTLvXo3f2AEMea4YDAQZnjXUoddpI0ZjK1w=; b=H p8Xz9YCWvnhJkCBV5JwEwGqDiIKCweGkOQXbzBEK+VXo5JjLt1i3IHGchszHqfnI dAxLU3jm+rjbEMvxG8qo3aRldqmxvxXLRtd59fztuT/OWeHh1vdXPcPOKlDp79gc 1hVziw7bQdoOrSrVSDxXyyv3c61wkTgB8YQ7BL5gcKds9NOvas/xCUNrURI/YNzV +2BoUygZ3DbGMcHmKYIjBKRvmjlms0ArxfEb7sL8t9US76VEBklMPxUP0vNZQoIx fKU/SZiVAxOHEI48/EMK+Sv+lYhvLQWXkK7YhzWRIXPA78dIeo+0WkGx03Qiacb8 SVRnAgQzsb3xRwZOL0bxA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrgedviedgvddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepkfffgggfuffvfhfhjggtgfesthejredttdefjeenucfhrhhomhepffhmihht rhihucfiuhhtohhvuceoughmihhtrhihsehguhhtohhvrdguvghvqeenucggtffrrghtth gvrhhnpeehfeefgfejieffhedtgfeuudefleekveekhedvieegvefggfeviefftddtjeej feenucffohhmrghinhepshhtrggtkhhovhgvrhhflhhofidrtghomhenucevlhhushhtvg hrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegumhhithhrhiesghhuthho vhdruggvvh X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 16 Jun 2023 22:40:11 -0400 (EDT) Content-Language: en-US In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:263496 Archived-At: Hi! This is a welcome addition, thanks for your efforts. On 14/06/2023 01:59, Morgan Smith wrote: > - "-1" "--no-color" ,@(ensure-list vc-git-log-switches) > + "-1" "--no-color" > + ;; The same as the default "medium" format but it doesn't > + ;; put spaces at the beginning of the body. This is so > + ;; we can grab this as the initial value when calling > + ;; log-view-modify-change-comment > + "--pretty=format:commit %H%nAuthor: %an %ae%nDate: %ad%n%n%B" > + ,@(ensure-list vc-git-log-switches) Does this improve the overall look of the log? Otherwise, we could remove the extra spaces when grabbing the log message (as long as we know how many were added), just like we do in log-edit-insert-changelog. But an even better solution, perhaps, would be to delegate to (vc-call-backend backend 'expanded-log-entry revision). Here's how it looked in my old, unfinished version of this feature: (defun log-view-modify-change-comment () "Edit the change comment displayed at point." (interactive) (let* ((file (and log-view-per-file-logs (log-view-current-file))) (revision (log-view-current-tag)) (backend (vc-responsible-backend (or file (car log-view-vc-fileset))))) (vc-modify-change-comment (list file) revision (vc-call-backend backend 'expanded-log-entry revision)))) It will need to fall back to log-view-extract-comment for backends that don't have this method defined, though. Such as CVS/RCS/SVN/SCCS. > -;; - Third, doing message editing in log-view might be a natural way to go > -;; about it, but editing any but the last commit (and even it, if it's > -;; been pushed) is a dangerous operation in Git, which we shouldn't make > -;; too easy for users to perform. ... > I found the hack located in vc-git-modify-change-comment' of setting > "GIT_SEQUENCE_EDITOR=:" on the internet and I don't understand how it > works. Apparently ":' is a shell built-in which "does nothing and then succeeds" (https://stackoverflow.com/a/29094904). "true" should also work. > Also I'm not really sure what the 'vc-dispatcher-browsing' function is > supposed to do but it's getting in my way. Like are 'diff-mode' buffers > a "directory browser buffer"? The docstring is 15 years old and never updated. If we can find a better wording, that would be great. > I don't understand the intent here. Check out the two places where it is called. > Also you'll noticed I deleted some comments in vc.el. That's mainly to > highlight them so we can talk about them now. Is there a good way to > stop users from editing remote commits? We could do something like this: take the remote branch associated with the current branch, and its top commit. And then, somehow, see if the commit to be edited is reachable by traversing up history. There likely are some known Git snippets out there on SO or general Internet that do these steps, but I haven't looked yet. Anyway, we could do this check first thing inside vc-git-modify-change-comment and, when the operation is dangerous, doubly prompt the user whether they want to proceed. Or, a more involved approach: add a new backend function which would do the check. Then log-view-modify-change-comment could abort earlier. > I feel like I should probably just keep working on this and get it more > polished and answer some of my own questions before posting here but I'm > getting kind of frustrated with this. I am many hours deep into this > problem. Please feel free to ask questions.