bug#18730: [PATCH] tildify.el: introduce a `tildify-space-string' variable

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> Deprecate `tildify-string-alist' variable and instead introduce
> a new `tildify-space-string' variable.  The alist was somehow
> complicated to use, both for users and in code, and did not work
> correctly with derived modes.  Instead of trying to make its
> handling even more complicated to fix the latter problem, replace
> it with `tildify-space-string' buffer-local variable.

The patch looks good.  Please install it with an appropriate ChangeLog
entry (and of course, the same text used as commit message).
See comments below,


        Stefan


> +  ;; If encoding allows use non-break space character as hard space, otherwise
> +  ;; use numeric entity (so we don't depend on   being defined).

You might like to add a FIXME in there deploring the fact that nxml-mode
doesn't derive from sgml-mode, which would save us from duplicating this code.

> +  (setq-local tildify-space-string
> +              (if (memq (coding-system-change-eol-conversion
> +                         buffer-file-coding-system nil)
> +                        (find-coding-systems-string " "))

Hmm... I would have used something more like

   (equal " " (decode-coding-string (encode-coding-string
                                     " " buffer-file-coding-system)
                                    buffer-file-coding-system))

> +(defcustom tildify-space-string nil

I think you could use " " as the default value so you can assume the var
only holds a string.

> -    (sgml-mode . " ")
> -    (html-mode . sgml-mode)

The new code uses "&#160" for those instead.  Is this change on purpose?

> +This variable is deprecated in favour of `tildify-space-string'
> +variable and takes effect only if `tildify-space-string' is not set.

Then add a (make-obsolete-variable 'tildify-string-alist
'tildify-space-string' "25.1").
And then check C-h v tildify-string-alist RET since the obsolescence info
in there will make part of the above text redundant.

> +	   (tilde (or tildify-space-string
> +		      (tildify--pick-alist-entry tildify-string-alist)
> +		      " "))

Since tildify-string-alist defaults to nil, I would make it take
precedence over tildify-space-string.


        Stefan