Subject: Re: Fwd: bug#47408: Etags support for Mercury [v0.4]

From: Eli Zaretskii <[email-masked]>
Date: Sat, 29 May 2021 11:01:01 +0300

[Please use Reply All to have the bug address on the CC list.]

> From: fabrice nicol <[email-masked]>
> Date: Sat, 15 May 2021 12:19:26 +0200
> 
> The confusion stems from an earlier discussion with a third party (I 
> forgot his name), who proposed to reuse '--no-defines' **and** 
> '--declarations' to replace Mercury-specific short options that I had 
> introduced. You followed this advice and so did I except for one detail 
> that I will outline below.
> 
> To make things clearer, I will rephrase the way I implemented this:
> 
> 1. There are **no longer** Mercury-specific short options -m/-M.
> 
> 2. As advised by this third party and your review comments, 
> '--declarations' is implemented in a Mercury-specific way (hence the - 
> correct - NEWS item about it, as well as a couple of sentences in the 
> man page, this is just what your review advised).
> 
> Using '--declarations', etags will tag not only Mercury language 
> declarations (strictly speaking) but also 'legacy declarations', i.e. 
> the old Prolog way (in actual terms, these are definitions in Mercury, 
> but there is unfortunately no '--definitions' etags options, so we must 
> do with what we have in store).

OK.  But the documentation patches you submitted seem to be somewhat
confusing: will Mercury declarations be tagged by default, or only
when --declarations is used?  This option isn't the default in etags,
so either the declarations are tagged by default (i.e. without using
this option) or only when --declarations is specified on the command
line.  You seem to say both, which I don't understand.

And I have several minor comments to the patch:

> +In Mercury, declarations start a line with "\|\fB:-\fP\|" and are tagged
> +by default.

Here, this is the confusing part about tagging Mercury declarations.

> ++++
> +** Etags support for the Mercury programming language (https://mercurylang.org).
> +** Etags command line option --declarations now has Mercury-specific behavior.
> +All Mercury declarations are tagged by default.
> +For compatibility with Prolog etags support, predicates and functions appearing
> +first in clauses will also be tagged if etags is run with '--declarations'.

And this as well.  It leaves me wondering what is the difference
between the results when using --declarations and not using it.

> --- /dev/null
> +++ b/lib-src/ChangeLog

We don't maintain ChangeLog files anymore in the repository, so what
you wanted to say there should be instead in the commit log message
(which you didn't include).  See CONTRIBUTE for more details about
formatting commit log messages.

> +      /* Disambiguate file names between Objc and Mercury */
> +      if (lang != NULL && strcmp(lang->name, "objc") == 0)
> +	test_objc_is_mercury(curfdp->infname, &lang);

Our C style conventions are to leave one space between the function's
name and the left parenthesis following it, as below:

   if (lang != NULL && strcmp (lang->name, "objc") == 0)
     test_objc_is_mercury (curfdp->infname, &lang);

Please make sure you use this style everywhere.

> diff --git a/test/manual/etags/merc-src/array.m b/test/manual/etags/merc-src/array.m
> new file mode 100644
> index 0000000000..0663c41087
> --- /dev/null
> +++ b/test/manual/etags/merc-src/array.m
> @@ -0,0 +1,3416 @@
> +%---------------------------------------------------------------------------%
> +% vim: ft=mercury ts=4 sw=4 et
> +%---------------------------------------------------------------------------%
> +% Copyright (C) 1993-1995, 1997-2012 The University of Melbourne.
> +% Copyright (C) 2013-2018 The Mercury team.
> +% This file is distributed under the terms specified in COPYING.LIB.

Adding test file(s) for Mercury is great, but we cannot have there
files under arbitrary license/copyright.  If COPYING.LIB mentioned
above specifies GPL or a compatible license, then it's okay for us to
distribute this file, but we need a copy of that COPYING.LIB file as
well.  If the license is not compatible with GPL, we cannot distribute
this file; in that case, please find some other test file, or provide
your own.

Thanks.