Changes to hi-lock before release.

Changes to hi-lock before release.

[David Koppelman]

I believe the following changes should be made to hi-lock before
release. The problem is that hi-lock can automatically add font-lock
keywords when a buffer is loaded, allowing for some mischief.  The
patch sets the loading of patterns from files off by default, but
by setting a variable the user can be prompted or can provide
a function to determine safety. The user can still add the patterns
by using a key sequence.

If font-lock-pattern safety-checking code becomes available later
I'll use that instead of these changes.


Index: hi-lock.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/hi-lock.el,v
retrieving revision 1.41
diff -u -p -r1.41 hi-lock.el
--- hi-lock.el	3 Feb 2007 17:26:01 -0000	1.41
+++ hi-lock.el	20 Apr 2007 19:00:30 -0000
@@ -3,7 +3,7 @@
 ;; Copyright (C) 2000, 2001, 2002, 2003, 2004,
 ;;   2005, 2006, 2007 Free Software Foundation, Inc.
 
-;; Author: David M. Koppelman, koppel@ee.lsu.edu
+;; Author: David M. Koppelman, koppel@ece.lsu.edu
 ;; Keywords: faces, minor-mode, matching, display
 
 ;; This file is part of GNU Emacs.
@@ -33,7 +33,8 @@
 ;;  will remove the highlighting.  Any existing face can be used for
 ;;  highlighting and a set of appropriate faces is provided.  The
 ;;  regexps can be written into the current buffer in a form that will
-;;  be recognized the next time the corresponding file is read.
+;;  be recognized the next time the corresponding file is read (when
+;;  file patterns is turned on).
 ;;
 ;;  Applications:
 ;;
@@ -60,6 +61,14 @@
 ;;
 ;;    (global-hi-lock-mode 1)
 ;;
+;;    To enable the use of patterns found in files (presumably placed
+;;    there by hi-lock) include the following in your .emacs file:
+;;
+;;    (setq hi-lock-file-patterns-policy 'ask)
+;;
+;;    If you get tired of being asked each time a file is loaded replace
+;;    'ask with a function that returns t if patterns should be read.
+;;
 ;;    You might also want to bind the hi-lock commands to more
 ;;    finger-friendly sequences:
 
@@ -115,6 +124,12 @@ calls."
   :type '(repeat symbol)
   :group 'hi-lock)
 
+(defvar 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.")
 
 (defgroup hi-lock-faces nil
   "Faces for hi-lock."
@@ -196,7 +211,7 @@ calls."
   "History of regexps used for interactive fontification.")
 
 (defvar hi-lock-file-patterns-prefix "Hi-lock"
-  "Regexp for finding hi-lock patterns at top of file.")
+  "Search target for finding hi-lock patterns at top of file.")
 
 (defvar hi-lock-archaic-interface-message-used nil
   "True if user alerted that `global-hi-lock-mode' is now the global switch.
@@ -283,17 +298,22 @@ called interactively, are:
   Remove highlighting on matches of REGEXP in current buffer.
 
 \\[hi-lock-write-interactive-patterns]
-  Write active REGEXPs into buffer as comments (if possible).  They will
+  Write active REGEXPs into buffer as comments (if possible).  They may
   be read the next time file is loaded or when the \\[hi-lock-find-patterns] command
   is issued.  The inserted regexps are in the form of font lock keywords.
-  (See `font-lock-keywords'.)  They may be edited and re-loaded with \\[hi-lock-find-patterns],
-  any valid `font-lock-keywords' form is acceptable.
+  (See `font-lock-keywords'.)  They may be edited and re-loaded with \\[hi-lock-find-patterns], 
+  any valid `font-lock-keywords' form is acceptable. When a file is
+  loaded the patterns are read if `hi-lock-file-patterns-policy is
+  'ask and the user responds y to the prompt, or if
+  `hi-lock-file-patterns-policy' is bound to a function and that
+  function returns t.
 
 \\[hi-lock-find-patterns]
   Re-read patterns stored in buffer (in the format produced by \\[hi-lock-write-interactive-patterns]).
 
-When hi-lock is started and if the mode is not excluded, the
-beginning of the buffer is searched for lines of the form:
+When hi-lock is started and if the mode is not excluded or patterns
+rejected, the beginning of the buffer is searched for lines of the
+form:
   Hi-lock: FOO
 where FOO is a list of patterns.  These are added to the font lock
 keywords already present.  The patterns must start before position
@@ -590,9 +610,18 @@ not suitable."
                 (setq all-patterns (append (read (current-buffer)) all-patterns))
               (error (message "Invalid pattern list expression at %d"
                               (line-number-at-pos)))))))
-      (when hi-lock-mode (hi-lock-set-file-patterns all-patterns))
-      (if (interactive-p)
-        (message "Hi-lock added %d patterns." (length all-patterns))))))
+      (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."

--- display.texi	12 Apr 2007 08:35:15 -0500	1.125
+++ display.texi	19 Apr 2007 21:36:51 -0500	
@@ -706,9 +706,16 @@ at point, with comment delimiters to pre
 program.  (This key binding runs the
 @code{hi-lock-write-interactive-patterns} command.)
 
-These patterns will be read the next time you visit the file while
+These patterns may be read the next time you visit the file while
 Hi Lock mode is enabled, or whenever you use the @kbd{M-x
-hi-lock-find-patterns} command.
+hi-lock-find-patterns} command. 
+
+Patterns are not read if the buffer's mode is listed in
+@code{hi-lock-exclude-modes}. Patterns are only used if
+@code{hi-lock-file-patterns-policy} is 'ask and the user responds yes
+to a prompt, or if @code{hi-lock-file-patterns-policy} is bound to a
+function and that function returns t. The function is called with the
+patterns as an argument.
 
 @item C-x w i
 @kindex C-x w i

Re: Changes to hi-lock before release.

[Chong Yidong]

David Koppelman <koppel@ece.lsu.edu> writes:

> I believe the following changes should be made to hi-lock before
> release. The problem is that hi-lock can automatically add font-lock
> keywords when a buffer is loaded, allowing for some mischief.  The
> patch sets the loading of patterns from files off by default, but
> by setting a variable the user can be prompted or can provide
> a function to determine safety. The user can still add the patterns
> by using a key sequence.

I double-checked the patch, and it looks right.  So I checked it in.
Thanks very much.

Re: Changes to hi-lock before release.

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Fri, 20 Apr 2007 15:29:31 -0400
> Cc: emacs-devel@gnu.org
> 
> David Koppelman <koppel@ece.lsu.edu> writes:
> 
> > I believe the following changes should be made to hi-lock before
> > release. The problem is that hi-lock can automatically add font-lock
> > keywords when a buffer is loaded, allowing for some mischief.  The
> > patch sets the loading of patterns from files off by default, but
> > by setting a variable the user can be prompted or can provide
> > a function to determine safety. The user can still add the patterns
> > by using a key sequence.
> 
> I double-checked the patch, and it looks right.  So I checked it in.

If we continue to check-in nonessential patches, how can we in good
faith try to convince Richard not to do the same?

Re: Changes to hi-lock before release.

[Johan Bockgård]

Eli Zaretskii <eliz@gnu.org> writes:

> If we continue to check-in nonessential patches, how can we in good
> faith try to convince Richard not to do the same?

** Local Variables:
** mode: hi-lock
** End:
** Hi-lock: ((eval . (launch-missiles)))

-- 
Johan Bockgård

Re: Changes to hi-lock before release.

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> I just clarified the documentation of this feature, as well
> as fixing Texinfo usage.  But I think some change is probably
> called for in the feature.
>
>     +(defvar hi-lock-file-patterns-policy 'never
>
> Shouldn't that be a defcustom?
> It should also have a risky-local-variable property
> since its value can be a function.

Let's just keep things as they are for now.

> Also, it seems absurd for an explicit call to hi-lock-find-patterns to
> check this variable.  If the user explicitly gives the command, it
> should be obeyed.  I think that hi-lock-file-patterns-policy should be
> tested only when hi-lock-find-patterns is called in other ways (such
> as by find-file).

What you suggested is indeed the behavior of hi-lock-find-patterns.

Re: Changes to hi-lock before release.

[Richard Stallman]

    ** Local Variables:
    ** mode: hi-lock
    ** End:
    ** Hi-lock: ((eval . (launch-missiles)))

What is that supposed to mean?
Are you reporting a bug?  If so, would you please
give a clear and proper report?

Re: Changes to hi-lock before release.

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     ** Local Variables:
>     ** mode: hi-lock
>     ** End:
>     ** Hi-lock: ((eval . (launch-missiles)))
>
> What is that supposed to mean?
> Are you reporting a bug?  If so, would you please
> give a clear and proper report?

Eli argued that this patch should not have been added to Emacs due to
the freeze.  John Bockgard is pointing out that it is important to add
it anyway, because it has security implications.

Re: Changes to hi-lock before release.

[Richard Stallman]

    >     +(defvar hi-lock-file-patterns-policy 'never
    >
    > Shouldn't that be a defcustom?
    > It should also have a risky-local-variable property
    > since its value can be a function.

    Let's just keep things as they are for now.

It makes no sense to put in a variable for user customization without
making it a defcustom.  Would someone please make it a defcustom?

Re: Changes to hi-lock before release.

[Richard Stallman]

    > Also, it seems absurd for an explicit call to hi-lock-find-patterns to
    > check this variable.  If the user explicitly gives the command, it
    > should be obeyed.  I think that hi-lock-file-patterns-policy should be
    > tested only when hi-lock-find-patterns is called in other ways (such
    > as by find-file).

    What you suggested is indeed the behavior of hi-lock-find-patterns.

I don't see it.  hi-lock-find-patterns takes no arguments
and it never bypasses checking hi-lock-file-patterns-policy.
How does it distinguish explicit M-x calls from calls that come from hooks?

We should fix this, but not before the release.

Re: Changes to hi-lock before release.

[Glenn Morris]

Richard Stallman wrote:

> It makes no sense to put in a variable for user customization without
> making it a defcustom.  Would someone please make it a defcustom?

It's now a defcustom (and has risky-local-var property).

Re: Changes to hi-lock before release.

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > Also, it seems absurd for an explicit call to hi-lock-find-patterns to
>     > check this variable.  If the user explicitly gives the command, it
>     > should be obeyed.  I think that hi-lock-file-patterns-policy should be
>     > tested only when hi-lock-find-patterns is called in other ways (such
>     > as by find-file).
>
>     What you suggested is indeed the behavior of hi-lock-find-patterns.
>
> I don't see it.  hi-lock-find-patterns takes no arguments
> and it never bypasses checking hi-lock-file-patterns-policy.
> How does it distinguish explicit M-x calls from calls that come from hooks?

      (when (and all-patterns
                 hi-lock-mode
                 (cond
                  ((eq this-command 'hi-lock-find-patterns) t)
                  ...

Re: Changes to hi-lock before release.

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Sat, 21 Apr 2007 15:26:45 -0400
> Cc: emacs-devel@gnu.org,
> 	Johan =?utf-8?Q?Bockg=C3=A5rd?= <bojohan+news@dd.chalmers.se>
> 
> Richard Stallman <rms@gnu.org> writes:
> 
> >     ** Local Variables:
> >     ** mode: hi-lock
> >     ** End:
> >     ** Hi-lock: ((eval . (launch-missiles)))
> >
> > What is that supposed to mean?
> > Are you reporting a bug?  If so, would you please
> > give a clear and proper report?
> 
> Eli argued that this patch should not have been added to Emacs due to
> the freeze.  John Bockgard is pointing out that it is important to add
> it anyway, because it has security implications.

For the record, I understood the implications even before Johan
pointed that out, thinking I didn't.  I still think we should not have
made that change.

Re: Changes to hi-lock before release.

[Richard Stallman]

    > I don't see it.  hi-lock-find-patterns takes no arguments
    > and it never bypasses checking hi-lock-file-patterns-policy.
    > How does it distinguish explicit M-x calls from calls that come from hooks?

	  (when (and all-patterns
		     hi-lock-mode
		     (cond
		      ((eq this-command 'hi-lock-find-patterns) t)
		      ...

How does that distinguish explicit M-x calls from calls that come from
hooks?

If that really does work, then the text I wrote in the manual needs
correcting.  Can you correct it?

Re: Changes to hi-lock before release.

[Richard Stallman]

    > It makes no sense to put in a variable for user customization without
    > making it a defcustom.  Would someone please make it a defcustom?

    It's now a defcustom (and has risky-local-var property).

Thanks.

Re: Changes to hi-lock before release.

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > I don't see it.  hi-lock-find-patterns takes no arguments
>     > and it never bypasses checking hi-lock-file-patterns-policy.
>     > How does it distinguish explicit M-x calls from calls that come from hooks?
>
> 	  (when (and all-patterns
> 		     hi-lock-mode
> 		     (cond
> 		      ((eq this-command 'hi-lock-find-patterns) t)
> 		      ...
>
> If that really does work, then the text I wrote in the manual needs
> correcting.  Can you correct it?

It works; I corrected the manual.

Re: Changes to hi-lock before release.

[Alan Mackenzie]

Hi, Richard, Chong and Emacs!

On Fri, Apr 20, 2007 at 02:06:14PM -0500, David Koppelman wrote:
> I believe the following changes should be made to hi-lock before
> release. The problem is that hi-lock can automatically add font-lock
> keywords when a buffer is loaded, allowing for some mischief.  The
> patch sets the loading of patterns from files off by default, but
> by setting a variable the user can be prompted or can provide
> a function to determine safety. The user can still add the patterns
> by using a key sequence.

> If font-lock-pattern safety-checking code becomes available later
> I'll use that instead of these changes.

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:

1: Start emacs -Q
2: Load the following file into Emacs with C-x C-f hilight-test.txt:
#########################################################################
Hi-lock: (("FIXME!!!" (0 (quote hi-yellow) t)))


This line contains a FIXME!!!
######################################################################### 
3: Do M-x hi-lock-mode

Nothing happens.  Zilch.  Sweet Fanny Adams.  What should happen is that
"FIXME!!!" become yellow.  It worked in Emacs 21.  At the very least,
the user should get something in the message area, such as "Hi-Lock:
auto highlighting of patterns disabled".  Surely?

I don't think it's reasonable to expect a normal user to go hunting
through the new Emacs manual or even the doc string (much less the
source code) to diagnose this absence of an informational message.  Many
of them are going to get angry, some of them are going to vent that
anger on help-gnu-emacs, and a fair number of them may feel like calling
us rude names - I, for one, wouldn't blame them.

The description of C-x w b (on page "Highlight Interactively") doesn't
seem to describe accurately what now happens when a file into which
patterns are written later gets visited.

The way I use hi-lock-mode is by having quite a lot of patterns at the
top of a personal log file which highlight things like dates, particular
filenames, "FIXME!!!", and so on.  I expect this kind of usage is quite
common, and have been used to it just working without hassle.  I think,
by contrast, that David (the writer of the patch) and most other people
here have been assuming that storing hi-lock patterns in a file is a
minor, barely used facility.

Why is the default for the new option `hi-lock-file-patterns-policy' set
to nil?  Surely it should be 'ask - MUST be 'ask, otherwise nobody will
know it's there.  Why isn't the value t effective for the variable?  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?  I think that hi-lock-mode is one of
relatively few of Emacs's advanced features that will appeal
particularly to non-programmers, and they're going to be less well
equipped to start writing defuns than a typical hacker.  Why are we
imposing this pain on them?

In the doc string fragment "If 'ask, prompt when patterns found in
buffer", I think that "prompt", used as an intransitive verb, is a bad
way of saying "ask the user"; it seems to be an abbreviation of "prompt
the user to anwer a question", but the essence of this fuller version is
the "answer", not the "prompt".  "prompt" (intr) seems to be
subliminally insulting - "nudge you, because you're too thick to see
this on your own".  So I think that should be changed too.

I also think it imperative that the option be a customization variable -
it will be jarring indeed for this isolated option NOT to be
customisable - it might even look like the variable was thrown in in a
hurry at the last moment.  ;-)  However, I don't feel up to learning how
to write defcustoms so late in the evening, so would somebody else be
willing to do this?

I think this situation urgently needs fixing.  For the other things,
I've started writing a patch.  I hope to be able to post it in
emacs-devel in a little over 12 hours time (I'm not hacker enough to go
24 hours without sleep).

-- 
Alan Mackenzie (Ittersbach, Germany).

Re: Changes to hi-lock before release.

[David Koppelman]

Alan Mackenzie <acm@muc.de> 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. 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.

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

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

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 <acm@muc.de> 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  <acm@muc.de>

	* 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).

Re: Changes to hi-lock before release.

[Richard Stallman]

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

Unsophisticated users don't usually put local variable bindings in
their files.  And if they put in local variable bindings for Hi Lock mode,
I think that proves they are sophisticated.  So I think we should
make the default `ask' for now.

Re: Changes to hi-lock before release.

[David Koppelman]

Richard Stallman <rms@gnu.org> writes:

> Unsophisticated users don't usually put local variable bindings in
> their files.  And if they put in local variable bindings for Hi Lock mode,
> I think that proves they are sophisticated.  So I think we should
> make the default `ask' for now.

I was thinking of users so unsophisticated they could not even use
custom to set hi-lock-file-patterns-policy to 'never.

Here are two patches. Both make the default 'ask, the first one, which
I prefer, also adds "(no is safe)" to the prompt.


2007-04-25  David Koppelman  <koppel@ece.lsu.edu>

	* hi-lock.el: (hi-lock-file-patterns-policy): Change
	default from 'never to 'ask; mention safety in custom text
	for function type.
	(hi-lock-find-patterns): Add "(no is safe)" to prompt.

Index: hi-lock.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/hi-lock.el,v
retrieving revision 1.44
diff -u -p -r1.44 hi-lock.el
--- hi-lock.el	22 Apr 2007 16:52:29 -0000	1.44
+++ hi-lock.el	25 Apr 2007 15:08:19 -0000
@@ -124,7 +124,7 @@ calls."
   :type '(repeat symbol)
   :group 'hi-lock)
 
-(defcustom hi-lock-file-patterns-policy 'never
+(defcustom hi-lock-file-patterns-policy 'ask
   "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
@@ -132,7 +132,7 @@ as first argument); if nil or `never' or
 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"))
+                 (function :tag "Function to check safety of file patterns"))
   :group 'hi-lock
   :version "22.1")
 
@@ -625,7 +625,8 @@ not suitable."
                   ((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? "))
+                   (y-or-n-p
+                    "Add patterns from this buffer to hi-lock (no is safe)? "))
                   (t nil)))
         (hi-lock-set-file-patterns all-patterns)
         (if (interactive-p)


*** Patch below same as one above except is does not add "(no is safe)"
to prompt.
	
2007-04-25  David Koppelman  <koppel@ece.lsu.edu>

	* hi-lock.el: (hi-lock-file-patterns-policy): Change
	default from 'never to 'ask; mention safety in custom text
	for function type.

Index: hi-lock.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/hi-lock.el,v
retrieving revision 1.44
diff -u -p -r1.44 hi-lock.el
--- hi-lock.el	22 Apr 2007 16:52:29 -0000	1.44
+++ hi-lock.el	25 Apr 2007 15:03:51 -0000
@@ -124,7 +124,7 @@ calls."
   :type '(repeat symbol)
   :group 'hi-lock)
 
-(defcustom hi-lock-file-patterns-policy 'never
+(defcustom hi-lock-file-patterns-policy 'ask
   "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
@@ -132,7 +132,7 @@ as first argument); if nil or `never' or
 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"))
+                 (function :tag "Function to check safety of file patterns"))
   :group 'hi-lock
   :version "22.1")

Re: Changes to hi-lock before release.

[Richard Stallman]

    I was thinking of users so unsophisticated they could not even use
    custom to set hi-lock-file-patterns-policy to 'never.

They are unlikely to put these local variables in their files,
so don't worry about them.

However, your changes are good, so would someone please install all of
them?