Stefan Monnier 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