From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Update js-mode function heading regexes Date: Fri, 29 Sep 2017 00:02:52 +0300 Message-ID: References: <7ad22b11-406f-1193-c948-faaf01268710@gmail.com> <365dbefc-1dee-c7c6-e9d5-c5a3b6fd0a9d@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Trace: blaine.gmane.org 1506643837 20444 195.159.176.226 (29 Sep 2017 00:10:37 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 29 Sep 2017 00:10:37 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Thunderbird/56.0 To: Adam Niederer , emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Sep 29 02:10:29 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dxisu-0004XW-Dk for ged-emacs-devel@m.gmane.org; Fri, 29 Sep 2017 02:10:28 +0200 Original-Received: from localhost ([::1]:33103 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dxit0-0007Lu-91 for ged-emacs-devel@m.gmane.org; Thu, 28 Sep 2017 20:10:34 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:37731) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dxisr-0007KU-E3 for emacs-devel@gnu.org; Thu, 28 Sep 2017 20:10:26 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dxisq-0006lk-6J for emacs-devel@gnu.org; Thu, 28 Sep 2017 20:10:25 -0400 Original-Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]:33953) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dxisp-0006kN-VE for emacs-devel@gnu.org; Thu, 28 Sep 2017 20:10:24 -0400 Original-Received: by mail-wm0-x243.google.com with SMTP id i131so147648wma.1 for ; Thu, 28 Sep 2017 17:10:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=JCrlkwyvWoD4I9HqmaPeJByo5j75jCebaVd3LI3ejgY=; b=nYUA+Rhhyf6f0TNK+KQsEpe+nmxStycqEKtwyGvLtBmyFWu0CihQFBZjiWMzNntn1J zbPmvt2TAUuy9Vit62xWgcR//z2gCQ4dafpJNUu/SV9S5NH/qfUre3zu3dhHe/kBsKcK bntmTRnbp8QIcNDgmHHOj8WxP1J5M31jbQ/5UeGKdBGQSx7+WkhFsouqZjqD1JrOxcpD 03G9Xj+7LKN4BrAOu0LwtYh59yS2UN4JeDzvtUqQ+7x0Qm1nJ+i1bYtEiOdg2fu2DRAV pfktJb5giy7WSuh612QusI+YPMrYE/li3M1tg5n1B7Tz0Q7InOvHETp1LFXoJc5RjJaR 0KlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=JCrlkwyvWoD4I9HqmaPeJByo5j75jCebaVd3LI3ejgY=; b=qGEqwoDthOzKmX8Meen7O0UUbb+M0E2vjBpyHn2nXKCY7766mKc3CzVCPleWGoI1HE vGEDLKjc/9RUHS9pGgxh6wd7b786QSPg+gOgp/1BuS5yVCJ4olJLuUNsfjQkJfod/mHE vt2S9WpJRmXD0P7kobN9KubG8+EpXdUXJu7GonKxPKzrdSTT2fOm+CwV2NPYQ2oh137S wZ91wQB7waNkJdZ5Y6A+HJ51Sg/hFC/65HIsqFC+IjTDCOMJAvZjSUsApSfenWawEnnB j/QFV0d9tRmxEPWnpcOgdJVqRsP/u9eMRqOK22Od0XS3JI5pzKQdKgpBNuOICUqgta98 Ndww== X-Gm-Message-State: AHPjjUiyV2PROzzPoNzDTI/cJktq7tMlACSTfA3bEYyWCO6sgf72CFEm uYSVlAuAVD/5yHClEqwlojnM3LD/IJI= X-Google-Smtp-Source: AOwi7QAHalPj0k9juPOryJLRad5zWhjwTILSCFyQVhghY1r9lwa7POZpjVsCQTUxQADvhD5nPopFcA== X-Received: by 10.80.133.205 with SMTP id q13mr4593489edh.50.1506643822725; Thu, 28 Sep 2017 17:10:22 -0700 (PDT) Original-Received: from [192.168.0.97] (catv-80-98-57-21.catv.broadband.hu. [80.98.57.21]) by smtp.googlemail.com with ESMTPSA id m10sm2466019eda.30.2017.09.28.17.10.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Sep 2017 17:10:21 -0700 (PDT) In-Reply-To: <365dbefc-1dee-c7c6-e9d5-c5a3b6fd0a9d@gmail.com> Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:400c:c09::243 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:218874 Archived-At: Hi Adam, Sorry for the late reply. On 6/19/17 10:04 AM, Adam Niederer wrote: > The file I included was primarily for visual evaluation of the changes. > Adding some automated tests is a good idea, though, so I've included an > updated patchset below. This also makes js--function-heading-2-re work > on async functions. Sometimes it's easier to evaluate the improvement just by looking at the tests, though it depends on how they are written, of course. To comment on the patch and your original post: - The change to heading-3-re is not mentioned, but it's fairly obvious. - I don't quite understand the description of the change in heading-2-re. Why not at least require a colon? >> Not much. Patch submissions via the bug tracker have higher odds of >> not being forgotten is they're not applied right away, though. >> >> Thanks for the patch. > > Would reposting these changes to bug-gnu-emacs be a good idea? By now it's looking even better than it did back then. As you can see, it's been lost in the mailing list. And I'd like for someone else to look at the patch, because it touches the area of js-mode than I've never had to deal with. > +(ert-deftest js-mode-highlight-function-names () > + "Ensure js--function-heading-1-re and js--function-heading-2-re match all > +possible function declarations and highlight the function names > accordingly." LGTM. > +(ert-deftest js-mode-accept-function-bindings () > + "Ensure js--function-heading-3-re matches all function declarations > of the > +form (var|let|const) foo = (async)? function." Regarding this test, can we check something higher level than whether js--function-heading-3-re matches? It's an implementation detail, after all.