From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id cJlbCdCB/2KSaQEAbAwnHQ (envelope-from ) for ; Fri, 19 Aug 2022 14:28:00 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id 4A9hCdCB/2IUXAEA9RJhRA (envelope-from ) for ; Fri, 19 Aug 2022 14:28:00 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id CE6173DF3C for ; Fri, 19 Aug 2022 14:27:59 +0200 (CEST) Received: from localhost ([::1]:43328 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oP16U-0006Af-Px for larch@yhetil.org; Fri, 19 Aug 2022 08:27:58 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39656) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oP15Y-0006AP-8U for emacs-orgmode@gnu.org; Fri, 19 Aug 2022 08:27:00 -0400 Received: from ciao.gmane.io ([116.202.254.214]:56380) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oP15W-0007nT-KW for emacs-orgmode@gnu.org; Fri, 19 Aug 2022 08:26:59 -0400 Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1oP15U-0004Fj-Ub for emacs-orgmode@gnu.org; Fri, 19 Aug 2022 14:26:56 +0200 X-Injected-Via-Gmane: http://gmane.org/ To: emacs-orgmode@gnu.org From: Max Nikulin Subject: Re: [PATCH v2] ol-info: Define :insert-description function Date: Fri, 19 Aug 2022 19:26:49 +0700 Message-ID: References: <87zgl1npow.fsf@localhost> <877d4flu3x.fsf@heagren.com> <87cze5e84m.fsf@localhost> <87tu7gkb4l.fsf@heagren.com> <87y1ws6o0c.fsf@localhost> <87k08bjw0t.fsf@heagren.com> <47248a4f-10aa-0980-c054-563f30c05aaa@gmail.com> <87mtd0gthe.fsf@heagren.com> <78b97c9e-fced-0ee4-f3f2-3cbe81080ffa@gmail.com> <87sfms9dx7.fsf@localhost> <87v8rmd53g.fsf@localhost> <871qu9xv8q.fsf@heagren.com> <0da49392-26c6-8ba3-f657-647522d59342@gmail.com> <87zggrg2om.fsf@heagren.com> <87edy3t8o0.fsf@localhost> <87tu6zf2o1.fsf@heagren.com> <871qu3rpt9.fsf@localhost> <8bbccdb4-52f4-b9b5-eb10-252bb15108ec@gmail.com> <87a68hn9es.fsf@localhost> <87zgg0q2kz.fsf@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Content-Language: en-US In-Reply-To: <87zgg0q2kz.fsf@localhost> Received-SPF: pass client-ip=116.202.254.214; envelope-from=geo-emacs-orgmode@m.gmane-mx.org; helo=ciao.gmane.io X-Spam_score_int: 28 X-Spam_score: 2.8 X-Spam_bar: ++ X-Spam_report: (2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_ADSP_CUSTOM_MED=0.001, FORGED_GMAIL_RCVD=1, FORGED_MUA_MOZILLA=2.309, FREEMAIL_FORGED_FROMDOMAIN=0.249, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, NICE_REPLY_A=-0.001, NML_ADSP_CUSTOM_MED=0.9, 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-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1660912079; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=wzEyekSnRhqsgMeSQA9OvGrPUu9T5rMskrKKUvkf/iI=; b=Y3m37YCbIUPq8mhxewJ6I5NYmCfA9ThGw5p0cUtEUDA2tToIinGs02iEynusfvg0wK7DSs z38waomkSqWY1zLPDsj98Ypfd5bjd9nIXguVgjrxKbToAtGkTIfFU91QWBDopZL/37OnDD l+UcNhZY3ItR4FrrsoQ/VD9SjWEEunNnT2QmzxJJ0T7O4meE1hd0X2ng2TiZuoihNjgvEB Hkh5ATeXhFx0eAEyW1of98QwHBC9V+Kqn/1l8no17hi+TvOXfjPQOuGc+EQoZt/+GRGVyG +5BirWrN26X8R10k9rcCvugIdEArpI9DWjwiSMZLI5VxTRf9OfLhHCK4/LJjJA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1660912079; a=rsa-sha256; cv=none; b=nKW/JLRwGLaxWk5H4AMyHgOU+FMSyI6yX88v/dYM0y0RkB4MdpUGX2Fbd8qt+qFHNPSm5g h/zZOZHAbL0PN2jmghy6p6ToIo91nTSGu0Kuea248+Tp55Z9+r+GltvqcDD7LBh9jN/LvA FERNxBNb8BEVlgviSTMStmM1T6quToMTNdM/TYBUKah8Y76jkxdKSh7e4QBW39j65o2n7h bkqWmvYJ+CBi0qg+NCzWHAErO+j2aarcqfoIMnso3Y1zV+EeYKRQCLIu2U64+Gq2uPZmi2 eKSKn5Cmz4Ur4llv6fDM5e5/gfxbIHEaXQN4zwW40vG0WWOcz9vVGoTGaY+iyA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: 2.85 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: CE6173DF3C X-Spam-Score: 2.85 X-Migadu-Scanner: scn1.migadu.com X-TUID: wX2tWByoP4PU On 19/08/2022 11:28, Ihor Radchenko wrote: > Max Nikulin writes: > >> +(defun org-info--link-file-node (path) >> + "Extract file name and node from info link PATH. >> + >> +Return cons consisting of file name and node name or \"Top\" if node >> +part is not specified. Components may be separated by \":\" or by \"#\"." > > It looks like the docstring does not match what the function actually > returns. It returns a cons, doesn't it? Is it confusing that separator for components is related to the argument? >> + (if (not path) >> + '("dir" . "Top") > > "dir" is not a file. Also, it is not very clear what "dir" is referring > to. Maybe you can add a comment pointing to `org-info-other-documents'? Try M-x info RET when you do not have *info* buffer and you get "(dir) Top" node. It is the directory of info files. If you run "info" without argument in shell you will get the same. >> + (string-match "\\`\\([^#:]*\\)\\(?:[#:]:?\\(.*\\)\\)?\\'" path) >> + (let* ((node (match-string 2 path)) >> + ;; `string-trim' modifies match > > Here and is several other places, including docstrings, please make sure > that the sentences end with "." and are separated with " ". It was supposed to be a brief phrase rather than complete sentence. >> + (cons >> + ;; Fallback to "org" is an arbirtrary choice >> + ;; and added because "(dir)filename" does not work as "filename". > > Should this be documented? Or at least mentioned that the behaviour is > undefined. And if it is undefined you should not test it in the tests. The purpose of test is to check that it does not signal some obscure error. I am unsure how to handle corner cases, so I am open to suggestions. Some considerations - `org-info--link-file-node' may return nil when link path is incomplete or some value that may help to avoid error handling code paths in callers. - does not look like a valid link but it may be handled like shell "info" command without argument, so I chose "(dir)" node. Elisp (info) without arguments however may display existing buffer. - certainly should be handled as (info "(dir)") - is invalid. Maybe (info "elisp") should be used instead. - I am unsure in my choice to open (info "(org) Tables"). Maybe it is better to treat it as "(dir) Tables" and so as "(dir) Top" node since "(dir) Top" is quite reasonable for with empty path. Thanks for the review.