emacs-devel
[Top][All Lists]
Advanced

[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: Ralph Amissah
Subject: Re: Ambrose Kofi Laing & Ralph Neelante Amissah [Emacs] sisu-mode.el - a major-mode for highlighting a structured text
Date: Sun, 21 Feb 2016 02:04:43 -0500
User-agent: Mutt/1.5.24 (2015-08-30)

Stefan, thank you for your work. Find attached a new diff against the
original Gnu sisu-mode.el

I hope I have understood what needed to be done. Sorry if there are
further things to address. I am glad to try; also please feel free to make
changes you think necessary before applying it. In the unlikely event that
I have reason to think otherwise, I can send you a subsequent request.

On Fri, Feb 19, 2016 at 01:42:19PM -0500, Stefan Monnier wrote:
> > 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.  ]
>

+;;; sisu-mode.el --- Major mode for SiSU markup text

thanks, corrected (not sure when or why it happened and have not gone back to
try find out, reverted)

> > -;; 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.
>

+;; Copyright (C) 2011, 2016 Free Software Foundation, Inc.

> > -;; 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.
>

+;; Author: Ralph Amissah & Ambrose Kofi Laing
+;; Keywords: text, syntax, processes, tools
+;; Version:   7.1.8

> > +;; 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.
>

+;; URL: 'http://www.sisudoc.org'

or should it follow the original format:
;; Home URL: SiSU:   http://www.sisudoc.org

> >  ;; License: GPLv3
>
> This is redundant.
>

removed

added

+;; with contributions from Kevin Ryde and Stefan Monnier

> > +      (cons "^group\{\\|^\}group"       'general-font-lock-red2)
>                       ^     ^
> These two backslashes do not do anything.
>

several such escapes removed

> > +;; 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.

got as far as this

+;; enables outlining for sisu
+(add-hook 'sisu-mode-hook
+         '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.
>

I did not get this quite

> > +;(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.
>

used ;;

> > -;;;###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.
>

reinstated


> > -  "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.
>

removed (interactive)

also modified the description to match what is in use elsewhere

   "Major mode for editing SiSU files.
+SiSU document structuring, publishing in multiple formats and search.
+URL `http://www.sisudoc.org/'

this description may be a bit terse, and I have no problem with some
modification, or with the removed repetition of "This major mode handles
SiSU markup"

> >    (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))
>
thanks

+;;;###autoload (add-to-list 'auto-mode-alist '("\\.ss[imt]\\'" . 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.
>
>

ok, removed

>         Stefan

I did a diff against the original file and put a few things back (e.g. the
triple curly braces).

Please let me know if there is more that needs to be done.

Thanks again,
Ralph

P.S. SiSU markup samples, for SiSU git contains some sample files
http://git.sisudoc.org/gitweb/?p=doc/sisu-markup-samples.git;a=summary
in particular here:
http://git.sisudoc.org/gitweb/?p=doc/sisu-markup-samples.git;a=tree;f=data/samples/current/en;hb=HEAD

P.P.S quick note to Kevin, thanks, (feel free to review the file)

Attachment: sisu-mode-7.1.8.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]