[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode
From: |
Stefan Monnier |
Subject: |
bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'. |
Date: |
Wed, 15 Oct 2014 10:35:34 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) |
Hi Michal,
Thanks for your patch. It looks like a good feature. See some
comments below.
Stefan
> It is configured via two new customize variables:
> `auto-tildify-pattern-alist' and `auto-tildify-check-envs'.
Could it use the existing tildify-pattern-alist?
> +Breaking line after a single-character words are forbidden
> +by Czech and Polish typography (and may be discouraged in other
> +languages), so `auto-tildify-mode' makes it easier to create
> +a typographically-correct documents.
Indeed, that's why we have fill-single-char-nobreak-p. You might like
to write the above text is such a way to make it clear what's the
difference between the two.
> + "Alist specifying whether to insert hard space at point.
IIUC this isn't quite right: "insert hard space" would imply adding
a space without changing anything else, whereas what you code does is to
replace a soft space by a hard space. So instead of "insert hard", I'd
say maybe "harden".
> +;;;###autoload
> +(defun auto-tildify ()
[...]
> +This function is meant to be used as a `post-self-insert-hook'."
Why is it autoloaded? Are users expected to add it to
post-self-insert-hook by hand?
> + (interactive)
Why is it interactive? Would it make sense to bind it to a key sequence
or to run it via M-x ?
> +;;;###autoload
> +(define-minor-mode auto-tildify-mode
I recommend you name it just `tildify-mode' (and generally just use
a "tildify-" prefix rather than "auto-tildify-").
For that, it would most likely make sense to rename tildify-mode-alist
(e.g. to tildify--mode-alist, tho another name could be even better
since this name doesn't actually say what it does).
> + (message "Hard space for %s is a single space character, auto-tildify
> won't have any effect." major-mode)
Please try to stay within the limits of 80 columns.
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Michal Nazarewicz, 2014/10/15
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'.,
Stefan Monnier <=
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Michal Nazarewicz, 2014/10/16
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Stefan Monnier, 2014/10/16
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Stefan Monnier, 2014/10/16
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Michal Nazarewicz, 2014/10/16
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Stefan Monnier, 2014/10/16
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Michal Nazarewicz, 2014/10/17
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Stefan Monnier, 2014/10/17
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Michal Nazarewicz, 2014/10/22
- bug#18730: [PATCH] tildify.el: Add `auto-tildify' and `auto-tildify-mode'., Stefan Monnier, 2014/10/24
- bug#18730: [PATCH] tildify.el: introduce a `tildify-space-string' variable, Michal Nazarewicz, 2014/10/28