From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#47408: Etags support for Mercury [v0.3] Date: Sun, 28 Mar 2021 16:11:51 +0300 Message-ID: <834kgvo220.fsf@gnu.org> References: <5ba2fec3-3f61-fb7e-35eb-7188fa6064a4@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28379"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 47408@debbugs.gnu.org To: fabrice nicol Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Mar 28 15:12:08 2021 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 1lQVD6-0007H1-Lt for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 28 Mar 2021 15:12:08 +0200 Original-Received: from localhost ([::1]:38118 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lQVD5-0002Kq-LK for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 28 Mar 2021 09:12:07 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:50772) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lQVD0-0002Kk-BX for bug-gnu-emacs@gnu.org; Sun, 28 Mar 2021 09:12:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:33988) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lQVD0-0003Qb-49 for bug-gnu-emacs@gnu.org; Sun, 28 Mar 2021 09:12:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lQVCz-00023J-RU for bug-gnu-emacs@gnu.org; Sun, 28 Mar 2021 09:12:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 28 Mar 2021 13:12:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47408 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 47408-submit@debbugs.gnu.org id=B47408.16169371147874 (code B ref 47408); Sun, 28 Mar 2021 13:12:01 +0000 Original-Received: (at 47408) by debbugs.gnu.org; 28 Mar 2021 13:11:54 +0000 Original-Received: from localhost ([127.0.0.1]:45534 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQVCr-00022w-OM for submit@debbugs.gnu.org; Sun, 28 Mar 2021 09:11:54 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:57758) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQVCp-00022k-LI for 47408@debbugs.gnu.org; Sun, 28 Mar 2021 09:11:52 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:52212) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lQVCk-0003GK-F6; Sun, 28 Mar 2021 09:11:46 -0400 Original-Received: from 84.94.185.95.cable.012.net.il ([84.94.185.95]:4147 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lQVCj-0008BR-NP; Sun, 28 Mar 2021 09:11:46 -0400 In-Reply-To: <5ba2fec3-3f61-fb7e-35eb-7188fa6064a4@gmail.com> (message from fabrice nicol on Sat, 27 Mar 2021 11:51:22 +0100) 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" Xref: news.gmane.io gmane.emacs.bugs:203158 Archived-At: > From: fabrice nicol > Date: Sat, 27 Mar 2021 11:51:22 +0100 > > I'm sending a new patch for this Mercury feature request. Thanks, I have some comments below. > >From 50f3f9a0d46d11d0ac096f79f0d5aa1bc17b7920 Mon Sep 17 00:00:00 2001 > From: Fabrice Nicol > Date: Sat, 27 Mar 2021 10:16:44 +0100 > Subject: [PATCH] Fixed regressions caused by Objc/Mercury ambiguous file > extension .m. Please accompany the changeset with a ChangeLog-style commit log message, you can see the style we are using via "git log" and also find some instructions in CONTRIBUTE. > .B \-V, \-\-version > Print the current version of the program (same as the version of the > emacs \fBetags\fP is shipped with). > +.TP > +.B \-, \-\-version > +Print the current version of the program (same as the version of the > +emacs \fBetags\fP is shipped with). Copy/paste mistake? or why are you duplicating the --version description? > +.TP > +.B \-M, \-\-no\-defines > +For the Mercury programming language, tag both declarations and > +definitions. Declarations start a line with \fI:\-\fP optionally followed by a > +quantifier over a variable (\fIsome [T]\fP or \fIall [T]\fP), then by > +a builtin operator like \fIpred\fP or \fIfunc\fP. > +Definitions are first rules of clauses, as in Prolog. > +Implies \-\-language=mercury. > +.TP > +.B \-m, \-\-declarations > +For the Mercury programming language, tag declarations as with \fB\-M\fP, but do not > +tag definitions. Implies \-\-language=mercury. This is not what Francesco Potortì suggested to do. He suggested that you use the existing options --no-defines and --declarations, but give them Mercury-specific meanings when processing Mercury source files. IOW, let's not introduce the new -m and -M shorthands for these options, and let's describe the Mercury-specific meaning of the existing options where they are currently described in etags.1. OK? > --- a/etc/NEWS > +++ b/etc/NEWS > @@ -93,6 +93,15 @@ useful on systems such as FreeBSD which ships only with "etc/termcap". > > * Changes in Emacs 28.1 > > +--- ^^^ This should be "+++", since you submitted the changes for the documentation as part of the changeset. > +** Etags support for the Mercury programming language (https://mercurylang.org). > +** New etags command line options '-M/-m' or --declarations/--no-defines'. > +Tags all Mercury declarations. For compatibility with Prolog etags support, > +predicates and functions appearing first in clauses will be tagged if etags is > +run with the option '-M' or '--declarations'. If run with '-m' or > +'--no-defines', declarations will be tagged but definitions will not. > +Both options imply --language=mercury. This should be amended for the changes in the options I described above. > +/* Define MERCURY_HEURISTICS_RATIO as it was necessary to disambiguate > + Mercury from Objective C, which have same file extensions .m */ This comment should explain how the value is used to disambiguate, so that people could decide what alternative value to use. > +static void test_objc_is_mercury(char *, language **); ^^ Our style is to leave one space between the function's name and the opening parenthesis. Please follow that here and elsewhere in your patch. > @@ -621,7 +629,6 @@ #define STDIN 0x1001 /* returned by getopt_long on --parse-stdin */ > "In Java code, all the tags constructs of C and C++ code are\n\ > tagged. (Use --help --lang=c --lang=c++ --lang=java for full help.)"; > > - > static const char *Cobol_suffixes [] = > { "COB", "cob", NULL }; > static char Cobol_help [] = Why remove this empty line? > static const char *Objc_suffixes [] = > - { "lm", /* Objective lex file */ > - "m", /* Objective C file */ > - NULL }; > + {"lm", > + "m", /* By default, Objective C will be assumed. */ > + NULL}; This loses the explanation that a .lm file is an ObjC lex file. > @@ -773,7 +792,6 @@ #define STDIN 0x1001 /* returned by getopt_long on --parse-stdin */ > 'TEXTAGS' to a colon-separated list like, for example,\n\ > TEXTAGS=\"mycommand:myothercommand\"."; > > - > static const char *Texinfo_suffixes [] = > { "texi", "texinfo", "txi", NULL }; > static const char Texinfo_help [] = Again, an empty line removed -- why? > + puts ("-m, --declarations\n\ > + For the Mercury programming language, only tag declarations.\n\ > + Declarations start a line with :- \n\ > + Implies --language=mercury."); > + > + puts ("-M, --no-defines\n\ > + For the Mercury programming language, include both declarations and\n\ > + definitions. Declarations start a line with :- while definitions\n\ > + are first rules for a given item, as for Prolog.\n\ > + Implies --language=mercury."); > + This should be merged with the existing description of the long options. > /* When the optstring begins with a '-' getopt_long does not rearrange the > non-options arguments to be at the end, but leaves them alone. */ > - optstring = concat ("-ac:Cf:Il:o:Qr:RSVhH", > + optstring = concat ("-ac:Cf:Il:Mmo:Qr:RSVhHW", > (CTAGS) ? "BxdtTuvw" : "Di:", > ""); As mentioned, let's not introduce -m and -M. > + case 'M': > + with_mercury_definitions = true; FALLTHROUGH; > + case 'm': > + { > + language lang = > + { "mercury", Mercury_help, Mercury_functions, Mercury_suffixes }; > + > + argbuffer[current_arg].lang = ⟨ > + argbuffer[current_arg].arg_type = at_language; > + } > + break; Shouldn't be needed anymore. > /* Etags options */ > - case 'D': constantypedefs = false; break; > + case 'D': constantypedefs = false; break; This whitespace change is for the worse: our conventions are to use mixed spaces-with-tabs style for indentation in C source files, not just spaces. > +static void test_objc_is_mercury(char *this_file, language **lang) Our style is to write function definitions like this: static void test_objc_is_mercury (char *this_file, language **lang) IOW, break the line between the return type and the function's name. > + FILE* fp = fopen(this_file, "r"); > + if (fp == NULL) return; No error/warning if the file couldn't be open? In any case, this leaks a FILE object: you open a file, but never close it. > + uint64_t lines = 1; > + uint64_t mercury_decls = 0; We don't use such types elsewhere in etags.c; why do you need them here? can you use intmax_t instead, as we do elsewhere? > + case '%': FALLTHROUGH; > + case ' ': FALLTHROUGH; > + case '\t': > + start_of_line = false; FALLTHROUGH isn't needed here, as there's no code under the first 2 'case' lines. > + /* Change the language from Objective C to Mercury */ Our style for comments is to end each comment with a period and 2 spaces, like this: /* Change the language from Objective C to Mercury. */ Please follow this style, here and elsewhere in the changeset. > + uint8_t decl_type_length = pos - origpos; Please use 'unsigned char' instead of uint8_t. > + if (( (s[len] == '.' /* This is a statement dot, not a module dot. */ > + || (s[len] == '(' && (len += 1)) > + || (s[len] == ':' /* Stopping in case of a rule. */ > + && s[len + 1] == '-' > + && (len += 2))) > + && (lastlen != len || memcmp (s, last, len) != 0) > + ) > + /* Types are often declared on several lines so keeping just > + the first line */ > + || is_mercury_type > + ) Please avoid parentheses alone on their lines. > diff --git a/lisp/speedbar.el b/lisp/speedbar.el > index 12e57b1108..63f3cd6ca1 100644 > --- a/lisp/speedbar.el > +++ b/lisp/speedbar.el > @@ -3534,6 +3534,8 @@ speedbar-fetch-etags-parse-list > speedbar-parse-c-or-c++tag) > ("^\\.emacs$\\|.\\(el\\|l\\|lsp\\)\\'" . > "def[^i]+\\s-+\\(\\(\\w\\|[-_]\\)+\\)\\s-*\C-?") > + ("^\\.m$\\'" . > + "\\(^:-\\)?\\s-*\\(\\(pred\\|func\\|type\\|instance\\|typeclass\\)+\\s+\\([a-z]+[a-zA-Z0-9_]*\\)+\\)\\s-*(?^?") > ; ("\\.\\([fF]\\|for\\|FOR\\|77\\|90\\)\\'" . > ; speedbar-parse-fortran77-tag) > ("\\.tex\\'" . speedbar-parse-tex-string) What about ObjC here? or are these keywords good for ObjC as well? Last, but not least: if you can, please provide a test file for the etags test suite, see test/manual/etags/. Thanks again for working on this.