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: vc-find-revision-no-save? Date: Sat, 11 Feb 2023 01:46:59 +0200 Message-ID: <4423f660-933d-bbcb-1432-76548dfbf08c@yandex.ru> 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="33380"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Cc: Emacs developers To: John Yates , Philip Kaludercic , Stefan Monnier , Stefan Kangas , schwab@linux-m68k.org, Juri Linkov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Feb 11 00:48:10 2023 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 1pQd7h-0008UY-LV for ged-emacs-devel@m.gmane-mx.org; Sat, 11 Feb 2023 00:48:09 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pQd6s-000349-UY; Fri, 10 Feb 2023 18:47:19 -0500 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 1pQd6q-00033v-4j for emacs-devel@gnu.org; Fri, 10 Feb 2023 18:47:16 -0500 Original-Received: from mail-ej1-x62d.google.com ([2a00:1450:4864:20::62d]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pQd6m-0004Dc-3i for emacs-devel@gnu.org; Fri, 10 Feb 2023 18:47:15 -0500 Original-Received: by mail-ej1-x62d.google.com with SMTP id qw12so19784925ejc.2 for ; Fri, 10 Feb 2023 15:47:06 -0800 (PST) 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:subject:date:message-id:reply-to; bh=AtPY+S+Yg7JjHVnljSCrbdVM57Mtysk8NBeshU8WGZ0=; b=YH+LHHltqTw6cQCalsilhMcqcC7vpbSL25vvOzS0szNFxEisqagQt6qsJe1HCyi3rx h7G4UyO9K2XEtCUgzV2CoXUBMzgmQFC2VPgPMFBYKfl2LjxGqi1SNOwMV0bGdJwfPxhj bLKpmhwq8dBxw444zrtINiEQ12nb4ngjDuhbJmqquiVuKwQsN1QCtZV6HBfHDA0Cx7Kv 9rBsNpLEvukhT42nItY/+buCdlrJqvxn0SK3Jlt181O9OMghW1Xyw6fi4by+yM2CMGw5 LZJ8/McHI3IQZ3/V1KIHqlQgaNQSTLF6m7sqmdd8DK5OubKgCVZScTNCxgTumGDLs67X BVwA== 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:subject:date:message-id :reply-to; bh=AtPY+S+Yg7JjHVnljSCrbdVM57Mtysk8NBeshU8WGZ0=; b=psBtA1DEEn0lNFfkYTSlRkAXTbEvZ8iFJyfchbWV/Nze5rFlL83T2V6Z3wHjyuZOgD lvHc6irmX81Yd2A1esXzBpIFTdwf1Z6m06DQORThvXxS5WMgY2XHfSb8UxweYO/ffpds SsYz9qC6iUdfSSwl9v8LSFIKJ32OqiWNA+ABKsNuCTJyHrmMv/k9dDDTUhADsxuWKr1u agV1wJn8Zcmi/kcS6IHoSkr3jUElJEAf1h9+rWSdAyt+ekZzI3JPWElc0Du0E03f3e3j v0E07a2UQrieYNvorS3k7kG8mmvPoemjTQdlgANoU8MuFMjl2FiGaquqegwrozfgVvlN NDGA== X-Gm-Message-State: AO0yUKXVwUqaY7Tju6PZD+7+u2NFgIkhCPZnSQZN2ojXfU8O0L8bjaje SqUR6zFogW1KFWU+0cu+6Us= X-Google-Smtp-Source: AK7set9//GH+wmD4bqAdglqVN2NNwpWl8bzQUpGNHnT/UGkOUuEbw2OdgPebBQdOAJA2im9kGmVDXA== X-Received: by 2002:a17:906:7c10:b0:8af:45d5:426f with SMTP id t16-20020a1709067c1000b008af45d5426fmr4324836ejo.33.1676072821975; Fri, 10 Feb 2023 15:47:01 -0800 (PST) Original-Received: from [192.168.0.2] ([46.251.119.176]) by smtp.googlemail.com with ESMTPSA id ov38-20020a170906fc2600b008af574e95d7sm1195546ejb.27.2023.02.10.15.47.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 10 Feb 2023 15:47:01 -0800 (PST) Content-Language: en-US In-Reply-To: Received-SPF: pass client-ip=2a00:1450:4864:20::62d; envelope-from=raaahh@gmail.com; helo=mail-ej1-x62d.google.com X-Spam_score_int: -26 X-Spam_score: -2.7 X-Spam_bar: -- X-Spam_report: (-2.7 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FORGED_FROMDOMAIN=0.249, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.248, NICE_REPLY_A=-1.149, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham 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-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:303117 Archived-At: Hi John, On 31/01/2023 13:57, John Yates wrote: > Towards the end of Oct 2022, each of you contributed to the above named > emacs-devel email thread. > > With issue #61071 on debbugs.gnu.org, I have attempted to address some > of the points mentioned in the email thread via: > > [PATCH 1/3] Refactor and document vc-find-revision caching > > I append the cover letter and commit message below. > > This is my very first attempt to contribute to Emacs. So far I have > received no feedback. If I am doing something wrong, please let me know. You've done everything correctly regarding the submission, but it seems like nobody highly interested in this particular addition has found the time to review it so far. Myself, I just saw the title (vc-timemachine) and skipped it. It wasn't obvious that some existing VC code was altered and needed reviewing. Sorry. We're also close to releasing Emacs 29, so it seemed like higher priority. Otherwise, though, it can be a good idea to ping a bug report a week or two after receiving to response. If nobody else does, the head maintainer usually ends up responding. I will also use this opportunity to complain about the lack of code review tools here. Some of the reviewers here might be able to quote attached patches inline, but I don't use Gnus, so that's not a real option. Just a lot of opening the patches externally and copying and pasting. We could move the review to EMBA, though: https://emba.gnu.org/emacs/emacs/-/merge_requests Now, to go back to the original thread you referred to, I mentioned vc-annotate, and you agreed that it has similar features but misses some stuff. Such as syntax highlighting. And editing support (is that necessary?). Overall, I think it might be better to add features to vc-annotate than add a very similar but different feature. Especially since it has unique features of its own, such as showing and being able to jump to a revision that last modified a given line. Or the one before it, etc. IME, that's usually more useful than going through a file history linearly. But that's my opinion. If that idea holds your interest, Stefan might also give a couple of recommendations, since as I recall he suggested something like that in the past. E.g. one of the ideas was to remove the sidebar text from the buffer text when processing, and instead store it in overlays or some such. Then put the buffer into a corresponding major mode which will apply font-lock and etc. But keep the (hidable) sidebar using overlays, for example. Or put it in the margin (also using overlays). And keep the navigation commands. The latter conflicts with having the buffer editable (if we're going to keep the one-char bindings). I haven't looked yet how it's solved with timemachine. The bindings might depend on whether the buffer is in read-only mode or not, though. Finally, some nits about the first patch: - It moves from the cache-by-default behavior to dont-cache-by-default. - It removes an existing user option without a deprecation period. - It adds a timemachine-related variable to vc.el (vc-tm--revision ?). Timemachine will be a separate package, right? The overall idea seems sound. But if we choose the route of improving vc-annotate, a revision cache will probably not help because we would be caching the 'git annotate' output instead. Thus making it specific to that feature only.