Re: Changes to hi-lock before release. PATCH.

[Alan Mackenzie]

'Morning, David!

On Tue, Apr 24, 2007 at 04:34:05PM -0500, David Koppelman wrote:

> Alan Mackenzie <address@hidden> writes:

> > Hi, Richard, Chong and Emacs!

> > Sorry to come into this thread so late.  I think there're serious
> > problems with the patched hi-lock-mode (I've just fired up the latest
> > pretest, 22.0.99): it treats users with less friendliness than they're
> > entitled to expect:

> It's all my fault. Had I attended to this sooner there would have been
> time to come up with something better for existing users.

Hey, I've been there, too!  So've lots of us.  It's a shared guilt kind
of thing, lets not obsess about it!  ;-)

> The reason for setting it to 'never rather than 'ask is because I felt
> it was worse to harass unsophisticated users with questions each time
> they visited a file than to temporarily deny users the patterns they
> already have in files.

OK.  Could I ask you to reconsider this?  I think that most of the time,
such files will be loaded by the people who created them - the Hi Lock
patterns are only meaningful within Emacs, and a typical work group will
be using a whole mix of different editors, so using these patterns to
exchange information between people only works up to a point.

On the other hand, people who've been using it since 4004 BC are going to
be stymied by the default 'never when they install Emacs 22.  People who
might start using C-x w b have yet one more (obscure) barrier to surmount
before they get it working.  With 'ask, the question is only put for
files which contain Hi Lock patterns, and I think this will happen by
accident only rarely.

I'd like to say here I LOVE Hi Lock mode.  It's one of my answers to the
question "what's so good about Emacs?"; some while ago a project manager
installed Emacs when I showed him how it could highlight the critical
lines of a multi-megabyte log where a sporadic failure was happening.  I
use the mode in my own personal log files, where I highlight dates (but
not walnuts ;), function names of the form "c-...", and various critical
words like "FIXME!!!", "OK", and so on.  It's a massive help.  Thanks for
writing the mode.

> In a future change I'm going to apply some kind of conservative safety
> test to the patterns and either reject or ask about those which fail
> the tests.

Things like :eval?  Most of the time, highlighting patterns aren't going
to cause trouble.

> I'm also thinking about specifying patterns in a .emacs file that would
> be applied to files with names matching a regexp (or passing some other
> test).

:-)

> > Why do I have to write

> > (defun return-t (arg) t)

> > , and set the hi-lock-file-patterns-policy to that instead?  Is this
> > some sort of security by obscurity?

> The security is against mischief in files visited by Emacs. Users edit
> their .emacs file at their own risk.

I don't think this is very helpful.  Many, perhaps nearly all, such
functions are just going to be returning t anyhow.  Is it not better just
to let users use t?

> The hi-lock-file-patterns-policy variable was made into a defcustom.

Yes.  Sorry about that noise.  I'd used the latest pretest without
bothering to check the CVS.  Sorry about the tone of my posting, too - I'd
spent the day at a place where people use Rational Rose, spell "process",
"review" and so on with capital letters, don't understand that short
precise words work better than tortuous rambling sentences with "system",
"deployment" and "...ation", and go around with glum blank expressions on
their faces.

Anyhow, here's the patch I promised last night.  It introduces t and
makes 'ask the default.  What do you think?



2007-04-25  Alan Mackenzie  <address@hidden>

        * hi-lock.el (hi-lock-file-patterns-policy): Allow the value t
        ("always-fontify"); make the default 'ask.
        (hi-lock-find-patterns): Use the new values of the above.



Index: hi-lock.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/hi-lock.el,v
retrieving revision 1.44
diff -c -r1.44 hi-lock.el
*** hi-lock.el  22 Apr 2007 16:52:29 -0000      1.44
--- hi-lock.el  25 Apr 2007 10:11:21 -0000
***************
*** 124,137 ****
    :type '(repeat symbol)
    :group 'hi-lock)
  
! (defcustom hi-lock-file-patterns-policy 'never
    "Specify when hi-lock should use patterns found in file.
! If `ask', prompt when patterns found in buffer; if bound to a function,
! use patterns when function returns t (function is called with patterns
! as first argument); if nil or `never' or anything else, don't use file
! patterns."
    :type '(choice (const :tag "Do not use file patterns" never)
                   (const :tag "Ask about file patterns" ask)
                   (function :tag "Function to check file patterns"))
    :group 'hi-lock
    :version "22.1")
--- 124,139 ----
    :type '(repeat symbol)
    :group 'hi-lock)
  
! (defcustom hi-lock-file-patterns-policy 'ask
    "Specify when hi-lock should use patterns found in file.
! If t, just use them; if 'ask, ask the user whether to use them;
! if nil or 'never or anything else but a function, ignore the file
! patterns; if bound to a function, use the value returned by the
! function, which should be one of the above values.  (The function
! is called with the patterns as argument)."
    :type '(choice (const :tag "Do not use file patterns" never)
                   (const :tag "Ask about file patterns" ask)
+                (const :tag "Always use file patterns" t)
                   (function :tag "Function to check file patterns"))
    :group 'hi-lock
    :version "22.1")
***************
*** 604,609 ****
--- 606,612 ----
    (interactive)
    (unless (memq major-mode hi-lock-exclude-modes)
      (let ((all-patterns nil)
+         policy
            (target-regexp (concat "\\<" hi-lock-file-patterns-prefix ":")))
        (save-excursion
        (save-restriction
***************
*** 618,635 ****
                  (setq all-patterns (append (read (current-buffer)) 
all-patterns))
                (error (message "Invalid pattern list expression at %d"
                                (line-number-at-pos)))))))
!       (when (and all-patterns
!                  hi-lock-mode
!                  (cond
!                   ((eq this-command 'hi-lock-find-patterns) t)
!                   ((functionp hi-lock-file-patterns-policy)
!                    (funcall hi-lock-file-patterns-policy all-patterns))
!                   ((eq hi-lock-file-patterns-policy 'ask)
!                    (y-or-n-p "Add patterns from this buffer to hi-lock? "))
!                   (t nil)))
!         (hi-lock-set-file-patterns all-patterns)
!         (if (interactive-p)
!             (message "Hi-lock added %d patterns." (length all-patterns)))))))
  
  (defun hi-lock-font-lock-hook ()
    "Add hi-lock patterns to font-lock's."
--- 621,637 ----
                  (setq all-patterns (append (read (current-buffer)) 
all-patterns))
                (error (message "Invalid pattern list expression at %d"
                                (line-number-at-pos)))))))
!       (when (and all-patterns hi-lock-mode)
!       (setq policy (if (functionp hi-lock-file-patterns-policy)
!                        (funcall hi-lock-file-patterns-policy all-patterns)
!                      hi-lock-file-patterns-policy))
!       (when (or (eq policy t)
!                 (and (eq policy 'ask)
!                  (y-or-n-p "Add patterns from this buffer to hi-lock? ")))
!                (hi-lock-set-file-patterns all-patterns)
!                (if (interactive-p)
!                    (message "Hi-lock added %d patterns."
!                             (length all-patterns))))))))
  
  (defun hi-lock-font-lock-hook ()
    "Add hi-lock patterns to font-lock's."


-- 
Alan Mackenzie (Ittersbach, Germany).