From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id aICUMqTXt2KQEgAAbAwnHQ (envelope-from ) for ; Sun, 26 Jun 2022 05:51:00 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id ALWVMqTXt2IfYQAA9RJhRA (envelope-from ) for ; Sun, 26 Jun 2022 05:51: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 791C82BC60 for ; Sun, 26 Jun 2022 05:50:59 +0200 (CEST) Received: from localhost ([::1]:46516 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o5JIY-0006cH-3u for larch@yhetil.org; Sat, 25 Jun 2022 23:50:58 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38136) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o5JHL-0006ZR-QM for emacs-orgmode@gnu.org; Sat, 25 Jun 2022 23:49:44 -0400 Received: from out1.migadu.com ([91.121.223.63]:41827) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o5JHI-0008Fh-GM for emacs-orgmode@gnu.org; Sat, 25 Jun 2022 23:49:42 -0400 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kyleam.com; s=key1; t=1656215376; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=atlUpI1mQ8SaIN5ggRq+9vE2n/rXkiFfPUqow8gdmL4=; b=zRsD9bfxy8ICfjSv8OZrue9VuooMINjCc7G/nymXcGV5hr0FhYw4IFKEAQNRSgjeFnPdeK +8khRNXIrrwoQjW+GC7Vcn/0TfKWuK7yk4bSTatmrvkI7NxLmSkZWh31FpX8c78RYZMN3H /hlUN0YbXYdeN1q8pxpsG/NBlaHdfp2TDlhccNpENFXDbWxbO8m1M+R7hmpGfDvWt6GXxV mJ+V/TvyB51dkM2ZJDdmzSg0oSZqZvZ50Nz1t45My5B9eHkV3bXhXoB7/jWPkz62wOKzMA FTLA7HqeNmPSD41I+4u9mdTC18woXmmeMKF9glWKZWGvo9SKDlmLqmm+XWaDsA== From: Kyle Meyer To: Ihor Radchenko , Daniel Fleischer Cc: emacs-orgmode@gnu.org, emacs@vergauwen.me Subject: Re: ox-latex table tabbing support. In-Reply-To: <87zgi0tcj6.fsf@localhost> References: <87wng5nno3.fsf@localhost-Mzo7eKN----2> <87zgi0tcj6.fsf@localhost> Date: Sat, 25 Jun 2022 23:49:31 -0400 Message-ID: <87k09413uc.fsf@kyleam.com> MIME-Version: 1.0 Content-Type: text/plain Received-SPF: pass client-ip=91.121.223.63; envelope-from=kyle@kyleam.com; helo=out1.migadu.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham 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=1656215459; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=atlUpI1mQ8SaIN5ggRq+9vE2n/rXkiFfPUqow8gdmL4=; b=O/HKEqalPnJ7Q4OTU6Pk39VeCpFdoxn8ktu5py5UOu+sBLT9fTIr+nVuelX+DjybGKOhPP kfWMAUhHIwYTPPfc6y9na1h82jYqOyAkTUugTMmXlX7ArpRHqaFoFxHpsf/CtzUBxXM/r9 r+W+dEBuQgfn8SVW4cho+3R7hovdJwkHODbc2cve0rrkGUbXbFFFI5GQJgAWlPLOAAY2IC LI5h+95uAr48wFo/vDQIF7Ds226OBIURzKIMA2Y8yWQ18gcqYRoCkKZpfpAdYlLoB3H5X+ V5ldqCBH3J3ymI/b583yX/4MnzVJzIUhRdZOyskn9hEUawI+GcMaXgdGvcOAbA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1656215459; a=rsa-sha256; cv=none; b=JgMTyiOmyCkdeXiB3EBA/P4TFMMql3+LmmDCyFPWucMhHKpR2W8pzrEeWmlU4P6eug6Iy1 mI3CysGlhe1RcbIc4nxyBYqNHFEcV/kaiHCXZf8f8JpT8ypCqfwGlF5IPL1Y/HKutqcZdn gGehgPRiwNuylaosW1F7KULoAavn3Pp9H/f+7rXj5c257sCAs0xDKeutry6C+aFFHnMUDM axf1YzLnOz2tQmZh3P3wQnFaXE7mWQkFVjNZGAbu5YdrfiVmbKyrctJ8Zi8jlWDtASIySF XxXLyADTDKwDAOKuoAJLmxiANz9uE3rULB7ctdwOToIBNvgkJQzLPYW97W40nQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=kyleam.com header.s=key1 header.b=zRsD9bfx; dmarc=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: -6.86 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=kyleam.com header.s=key1 header.b=zRsD9bfx; dmarc=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: 791C82BC60 X-Spam-Score: -6.86 X-Migadu-Scanner: scn0.migadu.com X-TUID: JcDhpqBWuET1 Ihor Radchenko writes: > Daniel Fleischer writes: > >> Thank you very much for the patch. It was merged to master in 4a0d951c. >> Also, you were added to the orgmode contributes at >> https://orgmode.org/worg/contributors.html. >> >> Thanks for the contribution. > > This commit triggers > > In org-latex--align-string-tabbing: > ox-latex.el:3713:54: Warning: Unused lexical variable `align' > > Looking at the code, it does not look like align variable serve any > purpose. Can someone please double check if the patch is working as > intended? Thanks for flagging this, Ihor. I was just glancing at this commit (4a0d951c6) due to seeing this warning. It's doing (let ((align ...)) (setq align ...)) where the align value is returned, so the align binding can be dropped altogether. Daniel, in addition to that, there are at least a few other issues with 4a0d951c6 that should be addressed: * the first line of the new docstrings should be a complete sentence. For example Return an appropriate LaTeX alignment string, for the latex tabbing environment. should be changed to something like Return alignment string for LaTeX tabbing environment. See (info "(elisp)Documentation Tips") * the indentation is off in several places, including the start of the docstrings. Please indent the code with, e.g., indent-region. * one of org-table--org-tabbing's parameters is a typo (contenst), which it looks like Ihor pointed out in an earlier review * comment typo: documets * several spots put opening and trailing parentheses on their own line That goes against the usual conventions of this repo: $ git grep '^ *)' '*.el' | wc -l 42 $ git grep '( *$' '*.el' | wc -l 17 * rather than doing something like (let ((x "")) (setq x ) ...) just do (let ((x )) ...) And consider whether it's worth adding a binding at all rather than inlining the code. As an extreme case, org-table--org-tabbing does (let ((output (format ...))) output) rather than (format ...)