Re: New ELPA package ada-lite

[Stephen Leake <stephen_leake@stephe-leake.org>]

[-- Attachment #1: Type: text/plain, Size: 4137 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I'd like to add a package ada-lite-mode to ELPA; code attached.  The
>> intent is to be a mode for Ada files that's easier to install than the
>> current ELPA ada-mode.
>
> Sounds good, as long as the other ada-mode will be modified to (require
> and) derive from (or extend) ada-lite.
>
> Currently, if both ada-mode and ada-lite packages are installed and
> activated, their autoloads will fight for the top spot of
> `auto-mode-alist`.

Hmm. I was assuming only one would be activated. In my current testing,
I have ada-mode disabled in package-load-list, so I have not run into
this problem.

I don't see how deriving ada-mode from ada-lite-mode helps here; there
are still conflicting entries in auto-mode-alist.

Almost all of ada-lite-mode is duplicated in ada-mode, so it would make
sense to use ada-lite-mode in ada-mode. However, that means there would
always be conficting entries in auto-mode-alist.

If a user wants both modes available so they can decide on a
file-by-file basis which one to use, they'll have to change
auto-mode-alist before visiting a file, or change the mode after
visiting, or something similar.

Are there other modes in ELPA (or MELPA?) that support the same file
extensions? There are several packages that mention "Python", for
example (some in MELPA). I don't have all of ELPA checked out; perhaps
you can do a grep-find for auto-mode-alist?

>
> A few more comments about the code:
>

Accepted unless commented on here; thanks for the detailed review.

>> (defun ada-lite-syntax-propertize (start end)
>>   "For `syntax-propertize-function'.
>> Assign `syntax-table' properties in region START .. END.
>> In particular, character constants are set to have string syntax."
>>   ;; (info "(elisp)Syntax Properties")
>>   ;;
>>   ;; called from `syntax-propertize', inside save-excursion with-silent-modifications
>>   (let ((inhibit-read-only t)
>> 	(inhibit-point-motion-hooks t))
>
> `inhibit-point-motion-hooks` is t (and obsolete) since Emacs-25.

Ah. I guess the byte-compiler doesn't report an obsolete warning here, because it's
in a let? Can that be improved?

> And `inhibit-read-only` should already be t since we're within
> `with-silent-modifications`.

Perhaps the doc string for syntax-propertize-function should state that
it is called from within with-silent-modifications.

And the doc string for with-silent-modifications should state that it
sets inibit-read-only t.

Patch attached.

>
>>     (goto-char start)
>>     (save-match-data
>
> Why?

I suspect this is left over from an earlier version of ada-mode.

> I think
>
>     (syntax-propertize-rules
>      ("[^[:alnum:])]\\('\\)[^'\n]\\('\\)"
>       (1 "\"'")
>       (2 "\"'")
>      ("[^[:alnum:])]\\('\\)'\\('\\)"
>       (1 "\"'")
>       (2 "\"'"))
>
> would do basically the same, but:
>
> - Is there a specific reason why you preferred to code it by hand
>   (speed maybe?)

I was unaware of syntax-propertize-rules.

> 
> - The second char in "\"'" is unused (it's only used for open/close
> parentheses-like thingies).

I guess "strings" are defined to always have the same start and end
delimiters.

> You could merge the two into:
>
>     (syntax-propertize-rules
>      ("[^[:alnum:])]\\('\\)\\(?:'\\|[^'\n]\\)\\('\\)"
>       (1 "\"")
>       (2 "\""))

I'll test with this.

>>   (set (make-local-variable 'syntax-propertize-function) #'ada-lite-syntax-propertize)
>>   (syntax-ppss-flush-cache (point-min));; reparse with new function
>
> Why/when have you found this explicit flush to be necessary?

When changing modes in a buffer, or just resetting the mode after
editing ada-lite-syntax-propertize.

You seem to be implying that syntax-ppss-flush-cache is already done by
the "set major mode" code?

>> (defun ada-lite-find-project (_dir)
>>   "If ada-lite-mode, return ada-lite project.
>> For `project-find-functions'"
>>   (and (eq major-mode 'ada-lite-mode)
>
> I recommend (derived-mode-p 'ada-lite-mode) instead?

That would be wrong for ada-mode if it derives from ada-lite-mode;
ada-mode assumes a different kind of project. 

-- 
-- Stephe

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: misc.diff --]
[-- Type: text/x-patch, Size: 1690 bytes --]

diff --git a/lisp/emacs-lisp/syntax.el b/lisp/emacs-lisp/syntax.el
index e1be301583..6b239c36c8 100644
--- a/lisp/emacs-lisp/syntax.el
+++ b/lisp/emacs-lisp/syntax.el
@@ -57,10 +57,12 @@ syntax-propertize-function
 It is the work horse of `syntax-propertize', which is called by things like
 Font-Lock and indentation.
 
-It is given two arguments, START and END: the start and end of the text to
-which `syntax-table' might need to be applied.  Major modes can use this to
-override the buffer's syntax table for special syntactic constructs that
-cannot be handled just by the buffer's syntax-table.
+It is given two arguments, START and END: the start and end of
+the text to which `syntax-table' might need to be applied.  It is
+called from within `with-silent-modifications'. Major modes can
+use this to override the buffer's syntax table for special
+syntactic constructs that cannot be handled just by the buffer's
+syntax-table.
 
 The specified function may call `syntax-ppss' on any position
 before END, but if it calls `syntax-ppss' on some
diff --git a/lisp/subr.el b/lisp/subr.el
index ca4d52535a..2b44a0316a 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4608,7 +4608,8 @@ with-silent-modifications
 than cosmetic ones, undo data may become corrupted.
 
 This macro will run BODY normally, but doesn't count its buffer
-modifications as being buffer modifications.  This affects things
+modifications as being buffer modifications.  `inhibit-read-only'
+and 'inhibit-modification-hooks' are both t.  This affects things
 like `buffer-modified-p', checking whether the file is locked by
 someone else, running buffer modification hooks, and other things
 of that nature."