[PATCH] Replace subst-char-in-region by subst-char-in-region

[PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

* gnus-sum.el (gnus-summary-update-mark): Use `subst-char-in-string'
rather than `subst-char-in-region' in order to be able to replace ASCII
char by UTF-8 ones.

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/ChangeLog   |    4 ++++
 lisp/gnus-sum.el |    6 +++++-
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 5d45055..78b196d 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -26,6 +26,10 @@
 
 2010-09-19  Julien Danjou  <julien@danjou.info>
 
+	* gnus-sum.el (gnus-summary-update-mark): Use `subst-char-in-string'
+	rather than `subst-char-in-region' in order to be able to replace ASCII
+	char by UTF-8 ones.
+
 	* gnus-html.el (gnus-html-prefetch-images): Use `url-retrieve' rather
 	than curl.
 	(gnus-html-image-fetched): Fix `gnus-html-put-image' call not setting
diff --git a/lisp/gnus-sum.el b/lisp/gnus-sum.el
index e09c3a5..c35cb25 100644
--- a/lisp/gnus-sum.el
+++ b/lisp/gnus-sum.el
@@ -10768,7 +10768,11 @@ If NO-EXPIRE, auto-expiry will be inhibited."
 	;; Go to the right position on the line.
 	(goto-char (+ forward (point)))
 	;; Replace the old mark with the new mark.
-	(subst-char-in-region (point) (1+ (point)) (char-after) mark)
+        (let ((to-insert
+               (subst-char-in-string (char-after) mark
+                                     (buffer-substring (point) (1+ (point))))))
+          (delete-region (point) (1+ (point)))
+          (insert to-insert))
 	;; Optionally update the marks by some user rule.
 	(when (eq type 'unread)
 	  (gnus-data-set-mark
-- 
1.7.1

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

Julien Danjou <julien@danjou.info> writes:

> * gnus-sum.el (gnus-summary-update-mark): Use `subst-char-in-string'
> rather than `subst-char-in-region' in order to be able to replace ASCII
> char by UTF-8 ones.

[...]

> +               (subst-char-in-string (char-after) mark
> +                                     (buffer-substring (point) (1+ (point))))))
> +          (delete-region (point) (1+ (point)))
> +          (insert to-insert))

Actually, this takes a large performance hit, so I don't think it should
be done.  Deleting/inserting something in a buffer moves the buffer gap
around, while subst-in-region does not.  If you're in a summary buffer
with 10K articles, and you hit `C', you'll get 10K buffer gap movements,
and it'll be very slow.

At least it was when I benchmarked that back in 1996.  :-)  (It used to
use delete/insert before that, if I remember correctly.)

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On Sun, Sep 19 2010, Lars Magne Ingebrigtsen wrote:

> Actually, this takes a large performance hit, so I don't think it should
> be done.  Deleting/inserting something in a buffer moves the buffer gap
> around, while subst-in-region does not.  If you're in a summary buffer
> with 10K articles, and you hit `C', you'll get 10K buffer gap movements,
> and it'll be very slow.

I'll take a look at performance.
Otherwise, if you do have an alternative, please let me know. :)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

Julien Danjou <julien@danjou.info> writes:

> I'll take a look at performance.

Ok.  It might not even be an issue any more -- I haven't benchmarked
stuff like that in a decade or more.

> Otherwise, if you do have an alternative, please let me know. :)

Nope.  It's why Gnus uses subst-char-in-region many places where other
solutions might be more natural.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

On Sun, Sep 19 2010, Lars Magne Ingebrigtsen wrote:

> Ok.  It might not even be an issue any more -- I haven't benchmarked
> stuff like that in a decade or more.

Well, tested on 5K messages. it only takes ~2s here to manipulate the
marks with "M C" or M-5000 M-u. :-)

> Nope.  It's why Gnus uses subst-char-in-region many places where other
> solutions might be more natural.

I grepped the sources, and anyhow, all other functions using
subst-char-in-region are using predefined replacement chars, not user
defined like in gnus-summary-update-mark. So they're safe (from me :-))
and should not bug out.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

Julien Danjou <julien@danjou.info> writes:

> Well, tested on 5K messages. it only takes ~2s here to manipulate the
> marks with "M C" or M-5000 M-u. :-)

How long did it take with the subst version?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

On Sun, Sep 19 2010, Lars Magne Ingebrigtsen wrote:

> How long did it take with the subst version?

Re-tested everything. Both versions took about the same time. If there's
a difference, I can't sense it.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

Julien Danjou <julien@danjou.info> writes:

> Re-tested everything. Both versions took about the same time. If there's
> a difference, I can't sense it.

Ok; great.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Reiner Steib]

On Sun, Sep 19 2010, Julien Danjou wrote:

> * gnus-sum.el (gnus-summary-update-mark): Use `subst-char-in-string'
> rather than `subst-char-in-region' in order to be able to replace ASCII
> char by UTF-8 ones.

Can you suggest which Unicode characters are nicer replacements for
the ASCII defaults (of gnus-FOO-mark)?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> Can you suggest which Unicode characters are nicer replacements for
> the ASCII defaults (of gnus-FOO-mark)?

I would guess that many people do not have the required unicode fonts
installed, which would make defaulting to using unicode characters by
default problematic.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

On Mon, Sep 20 2010, Reiner Steib wrote:

> Can you suggest which Unicode characters are nicer replacements for
> the ASCII defaults (of gnus-FOO-mark)?

This is what I currently use:

(setq gnus-score-over-mark ?↑)          ; ↑ ☀
(setq gnus-score-below-mark ?↓)         ; ↓ ☂
(setq gnus-ticked-mark ?⚑)
(setq gnus-dormant-mark ?⚐)
(setq gnus-expirable-mark ?♻)
(setq gnus-read-mark ?✓)
(setq gnus-del-mark ?✗)
(setq gnus-killed-mark ?☠)
(setq gnus-replied-mark ?⟲)
(setq gnus-forwarded-mark ?⤳)
(setq gnus-cached-mark ?☍)
(setq gnus-recent-mark ?★)
(setq gnus-unseen-mark ?✩)
(setq gnus-unread-mark ?✉)


-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

On Mon, Sep 20 2010, Lars Magne Ingebrigtsen wrote:

> I would guess that many people do not have the required unicode fonts
> installed, which would make defaulting to using unicode characters by
> default problematic.

Sure. We'll talk about that around 2020. ;)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Frank Schmitt]

Julien Danjou <julien@danjou.info> writes:

> On Mon, Sep 20 2010, Reiner Steib wrote:
>
>> Can you suggest which Unicode characters are nicer replacements for
>> the ASCII defaults (of gnus-FOO-mark)?
>
> This is what I currently use:
>
> (setq gnus-score-over-mark ?↑)          ; ↑ ☀
> (setq gnus-score-below-mark ?↓)         ; ↓ ☂
> (setq gnus-ticked-mark ?⚑)
> (setq gnus-dormant-mark ?⚐)
> (setq gnus-expirable-mark ?♻)
> (setq gnus-read-mark ?✓)
> (setq gnus-del-mark ?✗)
> (setq gnus-killed-mark ?☠)
> (setq gnus-replied-mark ?⟲)
> (setq gnus-forwarded-mark ?⤳)
> (setq gnus-cached-mark ?☍)
> (setq gnus-recent-mark ?★)
> (setq gnus-unseen-mark ?✩)
> (setq gnus-unread-mark ?✉)

Those look nice. Only the killed mark looks like the front of a bus for
me? (I'm running Gnus in a screen session on a remote server connected
via ssh, terminal is the almighty, rocking, rocking, rocking,
rxvt-unicode. Default font Terminus but for unicode I think it
automatically falls back to something sensible)

-- 
Have you ever considered how much text can fit in eighty columns?  Given that a
signature typically contains up to four lines of text, this space allows you to
attach a tremendous amount of valuable information to your messages.  Seize the
opportunity and don't waste your signature on bullshit that nobody cares about.

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Tassilo Horn]

On Monday 20 September 2010 11:02:47 Frank Schmitt wrote:

> > (setq gnus-killed-mark ?☠)
> 
> Those look nice. Only the killed mark looks like the front of a bus
> for me?

It's a skull with crossed bones!  That's so hell-of-a-pirate-cool, isn't
it?  I'm all in favour of adopting these marks.

Bye,
Tassilo

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Ted Zlatanov]

On Mon, 20 Sep 2010 10:40:25 +0200 Julien Danjou <julien@danjou.info> wrote: 

JD> This is what I currently use:

JD> (setq gnus-score-over-mark ?↑)          ; ↑ ☀
JD> (setq gnus-score-below-mark ?↓)         ; ↓ ☂
JD> (setq gnus-ticked-mark ?⚑)
JD> (setq gnus-dormant-mark ?⚐)
JD> (setq gnus-expirable-mark ?♻)
JD> (setq gnus-read-mark ?✓)
JD> (setq gnus-del-mark ?✗)
JD> (setq gnus-killed-mark ?☠)
JD> (setq gnus-replied-mark ?⟲)
JD> (setq gnus-forwarded-mark ?⤳)
JD> (setq gnus-cached-mark ?☍)
JD> (setq gnus-recent-mark ?★)
JD> (setq gnus-unseen-mark ?✩)
JD> (setq gnus-unread-mark ?✉)

On Mon, 20 Sep 2010 10:11:51 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> I would guess that many people do not have the required unicode fonts
LMI> installed, which would make defaulting to using unicode characters by
LMI> default problematic.

I suggest constructing the list on startup, based on `gnus-use-unicode'
or some such which the user can customize (I would default it to t but
warn if the font doesn't support those characters).

That should also produce nicer thread drawing characters.  I can use it
for the registry marks as well.

Ted

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Mon, Sep 20 2010, Ted Zlatanov wrote:

> I suggest constructing the list on startup, based on `gnus-use-unicode'
> or some such which the user can customize (I would default it to t but
> warn if the font doesn't support those characters).

I could install such a change, but how warn if the font doesn't support
those characters?

We could do:
  (defcustom gnus-read-mark (if (font-has-unicode-p) ?✓ ?R))

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Ted Zlatanov]

On Mon, 20 Sep 2010 16:57:07 +0200 Julien Danjou <julien@danjou.info> wrote: 

JD> On Mon, Sep 20 2010, Ted Zlatanov wrote:
>> I suggest constructing the list on startup, based on `gnus-use-unicode'
>> or some such which the user can customize (I would default it to t but
>> warn if the font doesn't support those characters).

JD> I could install such a change, but how warn if the font doesn't support
JD> those characters?

JD> We could do:
JD>   (defcustom gnus-read-mark (if (font-has-unicode-p) ?✓ ?R))

That's hard to read.  I think it's better to support sets of such
customizations, similar to how many people have structured thread
drawing characters.  So Gnus will do on startup:

(gnus-install-visuals gnus-visuals) ; try each visual set in turn

and then in gnus.el:

(setq gnus-visuals 'unicode 'ascii)
(setq gnus-visual-sets '((ascii ...) (unicode ...)))

and in your personal customizations:

(add-to-list 'gnus-visual-sets '(julien ...))
(setq gnus-visuals (cons 'julien gnus-visual-sets))

WDYT?  Of course, each set doesn't have to be complete, so they can
build on each other.  A set can have any number of:

- mark characters
- thread drawing characters
- registry mark characters
- others?

Ted

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 277 bytes --]

On Mon, Sep 20 2010, Ted Zlatanov wrote:

> WDYT?  Of course, each set doesn't have to be complete, so they can
> build on each other.  A set can have any number of:

Sounds like a good idea.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

This should be computed on the fly for each Emacs, or even each frame.
I'm now reading news via putty on my E72, and all the unicode chars are
question marks.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Ted Zlatanov]

On Mon, 20 Sep 2010 19:21:51 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> This should be computed on the fly for each Emacs, or even each frame.
LMI> I'm now reading news via putty on my E72, and all the unicode chars are
LMI> question marks.

gnus-install-visuals could also install font change hooks to prevent
problems when the font doesn't support the characters in windowed mode.

Specifically in your case over a terminal, I think you would have to set
the visuals to 'ascii since there's no way to tell what characters are
supported by Putty's font.

Ted

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> Specifically in your case over a terminal, I think you would have to set
> the visuals to 'ascii since there's no way to tell what characters are
> supported by Putty's font.

No...  but it'd be really nice if this could work without any
configuration.  I use Gnus under X, under xterm and via putty on my
phone, and Gnus should be usable in all those situations automatically,
without having to do anything in particular.

Perhaps (display-graphic-p) is useful for this?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

On Mon, Sep 20 2010, Lars Magne Ingebrigtsen wrote:

> Perhaps (display-graphic-p) is useful for this?

I don't think so. Unicode chars can be drawn on a terminal.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Ted Zlatanov]

On Mon, 20 Sep 2010 23:41:54 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> Ted Zlatanov <tzz@lifelogs.com> writes:
>> Specifically in your case over a terminal, I think you would have to set
>> the visuals to 'ascii since there's no way to tell what characters are
>> supported by Putty's font.

LMI> No...  but it'd be really nice if this could work without any
LMI> configuration.  I use Gnus under X, under xterm and via putty on my
LMI> phone, and Gnus should be usable in all those situations automatically,
LMI> without having to do anything in particular.

LMI> Perhaps (display-graphic-p) is useful for this?

No, you can't ask the terminal what fonts it's using or what characters
those fonts cover.  But remember that a) visual setting like these are
not critical, and b) users may prefer ASCII visuals even if they are
Unicode-capable.  In fact it's probably best to make the default ASCII
only and tell the users about it in the manual.

Ted

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> No, you can't ask the terminal what fonts it's using or what characters
> those fonts cover.

Is there any way to query it after you've displayed something?  Like
(is-char-at-point-displayed-properly-p)?  :-)

> But remember that a) visual setting like these are not critical, and
> b) users may prefer ASCII visuals even if they are Unicode-capable.
> In fact it's probably best to make the default ASCII only and tell the
> users about it in the manual.

Yeah.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: [PATCH] Replace subst-char-in-region by subst-char-in-region

[Ted Zlatanov]

On Tue, 21 Sep 2010 18:22:09 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> Ted Zlatanov <tzz@lifelogs.com> writes:
>> No, you can't ask the terminal what fonts it's using or what characters
>> those fonts cover.

LMI> Is there any way to query it after you've displayed something?  Like
LMI> (is-char-at-point-displayed-properly-p)?  :-)

Only in windowed mode, not in terminal mode AFAIK.  Maybe emacs-devel
will have better ideas?

Ted