[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Ambrose Kofi Laing & Ralph Neelante Amissah [Emacs] sisu-mode.el - a
From: |
Stefan Monnier |
Subject: |
Re: Ambrose Kofi Laing & Ralph Neelante Amissah [Emacs] sisu-mode.el - a major-mode for highlighting a structured text |
Date: |
Fri, 19 Feb 2016 13:42:19 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) |
> Please find attached sisu-mode.diff for sisu-mode.el
Thanks. I was about to apply it, but I think it's not quite right.
it's a bit hard to see what the patch does because it includes a lot of
whitespace changes, but even after accounting for those, there are some
changes which I don't think really want to install. See below.
Stefan
> -;;; sisu-mode.el --- Major mode for SiSU markup text
> +;;; sisu-mode.el --- a major-mode for highlighting a hierarchy structured
> text.
Why did you remove SiSU from the description?
[ Not opposed to it, but rather curious. ]
> -;; Copyright (C) 2011 Free Software Foundation, Inc.
> +;; Copyright (C): Free Software Foundation, Inc. (FSF) (GNU EMACS)
We want to keep all the years of publication in this line.
> -;; Author: Ambrose Kofi Laing (& Ralph Amissah)
> -;; Keywords: text, processes, tools
> -;; Version: 3.0.3
> +;; Author: Ralph Amissah & Ambrose Kofi Laing
> +;; Keywords: text, syntax, processes, tools
> +;; Version: 7.1.7 2015-12-26 Ralph Amissah,
The Version: pseudo-header should be of the form AA.BB.CC.DD... only.
> +;; URL:
> [http://git.sisudoc.org/gitweb/?p=code/sisu.git;a=blob;f=data/sisu/conf/editor-syntax-etc/emacs/sisu-mode.el;hb=HEAD]
Better than a link to the file would be a link to some "sisu-mode
project" web-page with further information than what's in this file.
> ;; License: GPLv3
This is redundant.
> + (cons "^group\{\\|^\}group" 'general-font-lock-red2)
^ ^
These two backslashes do not do anything.
> +;; enables outlining for sisu
> +(add-hook 'sisu-mode-hook
> + '(lambda ()
> + (outline-minor-mode)))
Please don't quote your lambdas.
Also (lambda () (outline-minor-mode)) is just a round-about way to say
#'outline-minor-mode.
Finally, it's generally preferred to put such code directly in the
sisu-mode function and leave the sisu-mode-hook nil by default.
> +;(define-key evil-normal-state-map (kbd ",0") (lambda() (interactive)
> (show-all)))
Single-semicolon comments are traditionally indented to comment-column
(e.g. column 40 or so), so you should probably use ";;" instead here.
> -;;;###autoload
> +;; Sisu & Autoload:
> (define-derived-mode sisu-mode text-mode "SiSU"
This ";;;###autoload" comment is the magic cookie used to auto-generate
sisu-mode-autoloads.el, so you don't want to remove it.
> - "Major mode for editing SiSU files.
> -SiSU (http://www.sisudoc.org/) is a document structuring and
> -publishing framework. This major mode handles SiSU markup."
> + "Major mode for editing SiSU files."
> + (interactive)
`define-derived-mode` already declares the function as interactive.
> (run-hooks 'sisu-mode-hook))
And `define-derived-mode` also automatically runs sisu-mode-hook for
you, so you don't need this either.
> -;;;###autoload (add-to-list 'auto-mode-alist '("\\.sisu\\'" . sisu-mode))
> +;; ##autoload
> +(add-to-list 'auto-mode-alist '("\\.sst\\'" . sisu-mode))
> +(add-to-list 'auto-mode-alist '("\\.ssm\\'" . sisu-mode))
> +(add-to-list 'auto-mode-alist '("\\.ssi\\'" . sisu-mode))
I think you want to use something like
;;;###autoload
(add-to-list 'auto-mode-alist '("\\.ss[itm]\\'" . sisu-mode))
or
;;;###autoload
(add-to-list 'auto-mode-alist '("\\.sst\\'" . sisu-mode))
;;;###autoload
(add-to-list 'auto-mode-alist '("\\.ssm\\'" . sisu-mode))
;;;###autoload
(add-to-list 'auto-mode-alist '("\\.ssi\\'" . sisu-mode))
> +;; 2011-07-12 Chong Yidong <address@hidden>
> +;;
> +;; Fix version numbers of sisu-mode, register-list, and windresize.
> +;;
> +;; 2011-07-08 Chong Yidong <address@hidden>
> +;;
> +;; sisu-mode.el: Add .sisu to auto-mode-alist using autoload cookie.
> +;; Minor doc fixes.
> +;;
> +;; 2011-07-06 Stefan Monnier <address@hidden>
> +;;
> +;; * sisu-mode.el (sisu-mode): Autoload.
> +;;
> +;; 2011-07-04 Stefan Monnier <address@hidden>
> +;;
> +;; Add sisu-mode.el. Update all.el licence.
This gets auto-added (from the Git log) when generating the ELPA
package, so we don't need it.
Stefan