replacing a function with another one

replacing a function with another one

[lee]

Hi,

is there a way to replace a function with another one other than
re-defining the original function?

Using (defalias) doesn´t seem to work for this.


The background is that I need a modified version of
(hi-lock-set-file-patterns):  The hi-lock-mode has different variables
for highlighting patterns with which it distinguishes between patterns
loaded from the file the buffer is visiting and patterns added through
actions of the user.  These two variables, `hi-lock-file-patterns´ and
`hi-lock-interactive-patterns´, are independent of each other.

When I visit a file with highlighting-patterns, the patterns are read
and applied and *not* added to the `hi-lock-interactive-patterns´
variable.  Consequently, when writing the current patterns to the
buffer, only those patterns are written which have been "recently"
defined by actions of the user.

To add new patterns, I am using wrapper functions, because I want to use
particular faces for the highlighting without needing to type them in
all the time.  When a new pattern is added, the wrapper functions remove
all patterns that have already been written to the buffer.  Then they
use (hi-lock-write-interactive-patterns) to put them back.  That way, I
don´t need to worry whether patterns I added are saved or not.

Since (hi-lock-write-interactive-patterns) writes only the patterns from
the `hi-lock-interactive-patterns´ variable to the buffer, all patterns
that were already there are forgotten.

To avoid this, I have a modified version of (hi-lock-set-file-patterns).
This version assigns the patterns read from the buffer to
`hi-lock-interactive-patterns´ so that they aren´t forgotten anymore.

The only way I have found to make the hi-lock-mode --- without modifying
the mode itself --- use my modified version of
(hi-lock-set-file-patterns) is to re-define this function.


Is this a way in which we are /supposed/ to customise emacs?  Should I
rather make a copy of the whole hi-lock-mode and modify it as I like?
I´d rather not do that (though I might have to because I want a number
of other things as well) because the original may receive fixes and
updates which would become difficult to keep track of.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Eli Zaretskii]

> From: lee <lee@yun.yagibdah.de>
> Date: Sat, 08 Mar 2014 20:43:36 +0100
> 
> is there a way to replace a function with another one other than
> re-defining the original function?

Did you consider defadvice?

Re: replacing a function with another one

[lee]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: lee <lee@yun.yagibdah.de>
>> Date: Sat, 08 Mar 2014 20:43:36 +0100
>> 
>> is there a way to replace a function with another one other than
>> re-defining the original function?
>
> Did you consider defadvice?

Thank you, that´s exactly what I´m looking for, and I got it to work :)


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> is there a way to replace a function with another one other than
> re-defining the original function?
>
> Using (defalias) doesn´t seem to work for this.

Although it's probably cleaner to use advices, `defalias' should also
work.  How did you call it?

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> is there a way to replace a function with another one other than
>> re-defining the original function?
>>
>> Using (defalias) doesn´t seem to work for this.
>
> Although it's probably cleaner to use advices, `defalias' should also
> work.  How did you call it?

The function is called by the hi-lock-mode itself.  I used defalias in
another mode.  It´s basically like this:


in mode A:

(defun mode-a-function-one (mode-a-function-two))


in mode B:

(defun mode-a-function-two-replacement)
(defalias 'mode-a-function-two 'mode-a-function-two-replacement)
(defun mode-b-function (mode-a-function-one))


It was possible to re-define 'mode-a-function with mode B, provided that
mode A was already loaded at the time the function was re-defined ---
which makes sense because otherwise there is nothing to re-define, and
mode A may even re-define the function of mode B when it´s loaded after
mode B is loaded.  The defalias somehow didn´t kick in.

Defadvice shines.  I wonder who invented that ...


And re-defining functions just like that?  Isn´t that a rather dangerous
and potentially troublesome feature?


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> Defadvice shines.  I wonder who invented that ...

The file header says it was Hans Chalupsky.

Note that defadvice has now been obsoleted by a new implementation:
nadvice.el.  You can do all of the stuff that defadvice can, but it has
cleaner and simpler syntax and semantics.  If this is new to you, you
may want to start learning with the new mechanism.

> And re-defining functions just like that?  Isn´t that a rather
> dangerous and potentially troublesome feature?

Generally you should try to avoid it as much as possible and only use it
when there is no other mean to reach what you want.

If you know what you are doing, it - in my experience - seldom causes
trouble.  Critical are changes in the sources that change syntax or
semantic of the original function, so expect that some of your advices
may stop working in future Emacs releases.


Regards,

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> Defadvice shines.  I wonder who invented that ...
>
> The file header says it was Hans Chalupsky.
>
> Note that defadvice has now been obsoleted by a new implementation:
> nadvice.el.  You can do all of the stuff that defadvice can, but it has
> cleaner and simpler syntax and semantics.  If this is new to you, you
> may want to start learning with the new mechanism.

Hm, that´s probably the reason why defadvice doesn´t appear in the
documentation.

So I guess should use advice-add instead --- and I can´t figure out how
to get that to work.  I need to access the argument passed to the
function the advice is for, too.  I tried to find examples, without
success.

What I have is:


(defadvice hi-lock-set-file-patterns (after lsl-hi-lock-set-file-patterns-advice activate)
  (setq hi-lock-interactive-patterns (ad-get-arg 0)))


How would I do the same with add-advice (or whatever is appropriate)?

> trouble.  Critical are changes in the sources that change syntax or
> semantic of the original function, so expect that some of your advices
> may stop working in future Emacs releases.

Yes, that´s exactly what I´m trying to avoid.  There is no way to detect
when something changes in hi-lock.el in such a way that what I´m doing
doesn´t work anymore, or is there?  Suddenly finding out that what I´m
trying to do doesn´t work anymore won´t be so great.

Taking a copy of hi-lock-mode and modify it isn´t a good solution,
either.  There may be changes to the mode which my own version wouldn´t
have.


One change I´m thinking about is keeping the highlighting-patterns in a
separate buffer and applying them to several buffers.  That way, you can
have several files that all use the same highlighting-patterns which
were generated on the fly, centralised for the whole project (or some
files of it).  That´ll probably require quite a few modifications.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:


> So I guess should use advice-add instead --- and I can´t figure out how
> to get that to work.  I need to access the argument passed to the
> function the advice is for, too.  I tried to find examples, without
> success.

IMHO the best description how the advice types work is in C-h f
add-function.

> What I have is:
>
>
> (defadvice hi-lock-set-file-patterns (after
> lsl-hi-lock-set-file-patterns-advice activate)
>   (setq hi-lock-interactive-patterns (ad-get-arg 0)))

> How would I do the same with add-advice (or whatever is appropriate)?

(advice-add
 'hi-lock-set-file-patterns :after
 (lambda (patterns)
   (setq hi-lock-interactive-patterns patterns)))

You can also give the piece of advice a name like in defadvice:

(advice-add
 'hi-lock-set-file-patterns :after
 (lambda (patterns)
   (setq hi-lock-interactive-patterns patterns))
 '((name . lsl-hi-lock-set-file-patterns-advice)))

But you can instead just name the function instead of using an anonymous
one:

(defun my-hi-lock-set-file-patterns-after-ad (patterns)
  (setq hi-lock-interactive-patterns patterns))

(advice-add 'hi-lock-set-file-patterns
	    :after 'my-hi-lock-set-file-patterns-after-ad)

The advantage is that advices are functions with the new implementation,
so referring to arguments and return values works transparently, instead
of the need of using obscure pseudo variables.

See C-h f add-function to learn how the other advice types work.

> Yes, that´s exactly what I´m trying to avoid.  There is no way to detect
> when something changes in hi-lock.el in such a way that what I´m doing
> doesn´t work anymore, or is there?

No, not direct way.

> Suddenly finding out that what I´m trying to do doesn´t work anymore
> won´t be so great.

Indeed.  But if you only rely on documented behavior (docstring), the
risk is low, since changes are backwards compatible most of the time.

> One change I´m thinking about is keeping the highlighting-patterns in a
> separate buffer and applying them to several buffers.  That way, you can
> have several files that all use the same highlighting-patterns which
> were generated on the fly, centralised for the whole project (or some
> files of it).  That´ll probably require quite a few modifications.

Don't hesitate to ask when you've questions, but I think you're on a
good way.


Regards,

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>>
>> (defadvice hi-lock-set-file-patterns (after
>> lsl-hi-lock-set-file-patterns-advice activate)
>>   (setq hi-lock-interactive-patterns (ad-get-arg 0)))
>
>> How would I do the same with add-advice (or whatever is appropriate)?
>
> (advice-add
>  'hi-lock-set-file-patterns :after
>  (lambda (patterns)
>    (setq hi-lock-interactive-patterns patterns)))
>
> You can also give the piece of advice a name like in defadvice:
> [...]

Thank you!  It seems rather complicated --- I´ll look into it tomorrow
and try to figure it out.

>> Suddenly finding out that what I´m trying to do doesn´t work anymore
>> won´t be so great.
>
> Indeed.  But if you only rely on documented behavior (docstring), the
> risk is low, since changes are backwards compatible most of the time.

/How/ the functions I´m advising do their work may change even when they
still do the same thing --- like a variable or an argument I´m using
might change or no longer be available ...

>> One change I´m thinking about is keeping the highlighting-patterns in a
>> separate buffer and applying them to several buffers.  That way, you can
>> have several files that all use the same highlighting-patterns which
>> were generated on the fly, centralised for the whole project (or some
>> files of it).  That´ll probably require quite a few modifications.
>
> Don't hesitate to ask when you've questions, but I think you're on a
> good way.

It´s somewhat frustrating when you have to figure out how things work
and stare at error messages that make you ask what they are supposed to
tell you --- but I´m learning :)

So far, I figured out this:


(defvar patterns-other-buffer nil)
(defvar original-buffer nil)

(defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
  (setq patterns-other-buffer (ad-get-arg 0))
  (ad-deactivate 'hi-lock-set-file-patterns)
  (pop-to-buffer original-buffer)
  (hi-lock-set-file-patterns patterns-other-buffer))

(defadvice hi-lock-find-patterns (before hi-lock-find-other-patterns activate)
  (my-test))

(defun my-test ()
  (interactive)
  (ad-activate 'hi-lock-set-file-patterns)
  (save-excursion
    (save-restriction
      (widen)
      (goto-char (point-min))
      (when (re-search-forward "^// ext-fontify-defs: " nil t)
	(message "found marker")
	(let ((filename (thing-at-point 'filename)))
	  (message "filename at %d: %s/%s for buffer %s" (point) filename (thing-at-point 'filename) (buffer-name original-buffer))
	  (when filename
	    (when (file-exists-p filename)
	      (setq original-buffer (current-buffer))
	      (find-file filename)
	      (set-auto-mode-0 'hi-lock-mode t))))))))


This actually works, i. e. it lets me save hi-lock-mode
highlighting-patterns to an arbitrary file and put a line like "//
ext-fontify-defs: name-of-file-with-patterns" into the file to load the
highlighting-patterns from the file the name of which is specified in
that line.  The patterns are then applied.

But I don´t like this.  It´s a really terrible hack.

Or is this the way to go (letting aside the two obvious glitches)?

And I would end up with two defadvices on the same function ... but as
far as I understand the documentation, that should work fine.  Hm ...


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> Thank you!  It seems rather complicated --- I´ll look into it tomorrow
> and try to figure it out.

Actually, it's only abstract, but quite simple.

About the original question you raised (not forgetting patterns), would
something like this do what you want:

(advice-add
 'hi-lock-write-interactive-patterns :around
 ;; also include `hi-lock-file-patterns'
 (lambda (f &rest args)
   (let ((hi-lock-interactive-patterns
	  (append hi-lock-interactive-patterns
		  hi-lock-file-patterns))))
   (apply f args)))

> It´s somewhat frustrating when you have to figure out how things work
> and stare at error messages that make you ask what they are supposed to
> tell you --- but I´m learning :)
>
> So far, I figured out this:
>
>
> (defvar patterns-other-buffer nil)
> (defvar original-buffer nil)
>
> (defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
>   (setq patterns-other-buffer (ad-get-arg 0))
>   (ad-deactivate 'hi-lock-set-file-patterns)
>   (pop-to-buffer original-buffer)
>   (hi-lock-set-file-patterns patterns-other-buffer))
>
> (defadvice hi-lock-find-patterns (before hi-lock-find-other-patterns activate)
>   (my-test))
>
> (defun my-test ()
>   (interactive)
>   (ad-activate 'hi-lock-set-file-patterns)
>   (save-excursion
>     (save-restriction
>       (widen)
>       (goto-char (point-min))
>       (when (re-search-forward "^// ext-fontify-defs: " nil t)
> 	(message "found marker")
> 	(let ((filename (thing-at-point 'filename)))
> 	  (message "filename at %d: %s/%s for buffer %s" (point) filename (thing-at-point 'filename) (buffer-name original-buffer))
> 	  (when filename
> 	    (when (file-exists-p filename)
> 	      (setq original-buffer (current-buffer))
> 	      (find-file filename)
> 	      (set-auto-mode-0 'hi-lock-mode t))))))))
>
>
> This actually works, i. e. it lets me save hi-lock-mode
> highlighting-patterns to an arbitrary file and put a line like "//
> ext-fontify-defs: name-of-file-with-patterns" into the file to load the
> highlighting-patterns from the file the name of which is specified in
> that line.  The patterns are then applied.
>
> But I don´t like this.  It´s a really terrible hack.

I'm not sure I completely understand what you are trying to do.

Generally, you could use directory local variables for storing your
patterns, but these would of course not be automatically updated, what
you presumably want.

Nitpick: instead of activating and deactivating pieces of advice, define
a variable and use it as a flag.  The flag says whether the advice
should kick in.  In the advice's body, look at the variable and decide
what to do.

> And I would end up with two defadvices on the same function ... but as
> far as I understand the documentation, that should work fine.  Hm ...

Yes, that should work fine.  But you can anytime combine any number of
advices into one around advice.

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> Thank you!  It seems rather complicated --- I´ll look into it tomorrow
>> and try to figure it out.
>
> Actually, it's only abstract, but quite simple.

Too abstract, maybe:  I understand by removing abstractions and don´t
know how else I could understand something.  I can only do it the other
way round, i. e. understand something first and then come up with
abstractions.  That´s a barrier which has its merits.

> About the original question you raised (not forgetting patterns), would
> something like this do what you want:
>
> (advice-add
>  'hi-lock-write-interactive-patterns :around
>  ;; also include `hi-lock-file-patterns'
>  (lambda (f &rest args)

What does (f &rest args) do or mean?

>    (let ((hi-lock-interactive-patterns
> 	  (append hi-lock-interactive-patterns
> 		  hi-lock-file-patterns))))

So this makes one list from both variables (lists) and assigns it to one
of them.  Then, I suppose, the intention is to have
hi-lock-write-interactive-patterns write them --- i. e. combine both
lists just before writing them and otherwise keep them separate.  This
would be better than my approach because it avoids duplication.

I´ll have to try that out.

>    (apply f args)))

f args?

hi-lock-write-interactive-patterns doesn´t have any arguments.

>> (defvar patterns-other-buffer nil)
>> (defvar original-buffer nil)
>>
>> (defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
>>   (setq patterns-other-buffer (ad-get-arg 0))
>>   (ad-deactivate 'hi-lock-set-file-patterns)
>>   (pop-to-buffer original-buffer)
>>   (hi-lock-set-file-patterns patterns-other-buffer))
>>
>> (defadvice hi-lock-find-patterns (before hi-lock-find-other-patterns activate)
>>   (my-test))
>>
>> (defun my-test ()
>>   (interactive)
>>   (ad-activate 'hi-lock-set-file-patterns)
>>   (save-excursion
>>     (save-restriction
>>       (widen)
>>       (goto-char (point-min))
>>       (when (re-search-forward "^// ext-fontify-defs: " nil t)
>> 	(message "found marker")
>> 	(let ((filename (thing-at-point 'filename)))
>> 	  (message "filename at %d: %s/%s for buffer %s" (point) filename (thing-at-point 'filename) (buffer-name original-buffer))
>> 	  (when filename
>> 	    (when (file-exists-p filename)
>> 	      (setq original-buffer (current-buffer))
>> 	      (find-file filename)
>> 	      (set-auto-mode-0 'hi-lock-mode t))))))))
>>
>>
>> This actually works, i. e. it lets me save hi-lock-mode
>> highlighting-patterns to an arbitrary file and put a line like "//
>> ext-fontify-defs: name-of-file-with-patterns" into the file to load the
>> highlighting-patterns from the file the name of which is specified in
>> that line.  The patterns are then applied.
>>
>> But I don´t like this.  It´s a really terrible hack.
>
> I'm not sure I completely understand what you are trying to do.

I´m trying to make an extension to hi-lock-mode.

The mode keeps the highlighting-patterns in the same buffer as the text
that is being highlighted.  Instead, I want to be able to keep the
highlighting-patterns in a separate file.  From the buffer that has the
text which is to be highlighted, I want to refer to this separate file
so that the highlighting-patterns kept in the separate file are used to
highlight the text in the buffer.

Think of it as a form of '#include "highlighting.patterns"'.


To achieve this, I´m inserting my own function before
hi-lock-find-patterns.  My own function searches for the equivalent of
the '#include "highlighting.patterns"' statement.  When it finds one, it
visits[1] the specified file (like "highlighting.patterns").  That puts
this file into the current buffer.

Since hi-lock-find-patterns reads the highlighting-patterns from the
current buffer, it now reads them from the file I have included rather
than from the original buffer.  And hi-lock-find-patterns calls
hi-lock-set-file-patterns, giving the patterns as an argument to it so
that hi-lock-set-file-patterns can apply the highlighting to the current
buffer.

At that point, the current buffer is the included file and not the
original buffer I want the highlighting-patterns applied to.  Therefore,
I need to get a copy of the patterns, switch back to the original buffer
and apply the patterns from my copy.  To get a copy of the patterns and
to apply them to the original buffer, I use hi-lock-set-file-patterns:

So I first advise hi-lock-set-file-patterns to get a copy of the
patterns.  Then I disable the advice, switch to the original buffer and
run the non-advised version of hi-lock-set-file-patterns again to
finally apply my copy of the patterns to the buffer I want them applied
to.

It´s self-modifying code, a creative (ab)use of defadvice.

I think it´s an awkward way to do this and a terrible hack: I´m cheating
an "unexpected" buffer underneath hi-lock-find-patterns to read
"unexpected" patterns.  This isn´t necessarily too bad, but much worse,
I´m bending hi-lock-set-file-patterns in a weird way.

> Generally, you could use directory local variables for storing your
> patterns, but these would of course not be automatically updated, what
> you presumably want.

Hmm ...  First I should make the variables I´m using to remember what
was the original buffer and to have a copy of the patterns local.  Then
I should somehow follow your approach to avoid duplication of the
patterns.

Another step is to modify things so that when adding a new pattern, the
new pattern is added to the included file rather than to the original
file.  Avoiding the duplication might make this easier.

> Nitpick: instead of activating and deactivating pieces of advice, define
> a variable and use it as a flag.  The flag says whether the advice
> should kick in.  In the advice's body, look at the variable and decide
> what to do.

That´s a good idea.  I don´t need to bend hi-lock-set-file-patterns so
much when I use a variable.  And I can probably use add-advice because I
don´t need to disable the advice anymore.


[1]: Another question:

To get patterns from a separate file, I´m using find-file.  This is
probably not very efficient:  I suspect that the file is actually loaded
every time, with all what that implies.  So when I have X files that
want to read highlighting-patterns from the same file, that same file is
loaded and processed X times.

It would be more efficient to load and to process the file only once.  I
don´t know how to go about that, though.  Is there a function that tells
me for a given file which buffer is visiting it?  I could write one that
goes through the buffer list and returns the buffer visiting the file,
but maybe there is something better for this.

As to processing the file only once, I´d have to somehow check whether
the currently available copy of patterns is from the file that is to be
included.  If it is, just apply the copy, if not, read the file.

But then, suppose I have X files each of which include patterns-A and
patterns-B:  This caching strategy would fail, and patterns-A and
patterns-B would be processed X times.

Can I somehow have a (global) "collection" of included files and their
patterns?  In that case, once patterns-A has been processed, the
patterns would be available and could be applied to any file which
includes patterns-A, no matter in which order the pattern-files are
included.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> > (advice-add
> >  'hi-lock-write-interactive-patterns :around
> >  ;; also include `hi-lock-file-patterns'
> >  (lambda (f &rest args)

> What does (f &rest args) do or mean?

From the doc of `add-function', this is how :around advices FUNCTION are
called:

  (lambda (&rest r) (apply FUNCTION OLDFUN r))

Note that the advice FUNCTION will be called with an additional (the
first) argument, which will be bound to the original function when the
advice is called.  This way, you have direct access to the original
function through that binding in your advice, and you can call it with
funcall or apply.  This "mechanism" is the replacement for the old
ad-do-it.

In my example, FUNCTION is (lambda (f &rest args) ...), which means that
f will be bound to OLDFUN, i.e. the original definition of
`hi-lock-write-interactive-patterns', and args will be bound to r, which
will be the list of arguments with which
`hi-lock-write-interactive-patterns' gets called.

> >    (apply f args)))
>
> f args?
>
> hi-lock-write-interactive-patterns doesn´t have any arguments.

So, the function will always be called without arguments, which means
that args will be bound to the empty list - no problem.

So why did I write it that way?  Because it works with any argument
list.  Instead of specifying the original argument list in the advice,
I just use  &rest args  and apply the original function to args when I
don't need to know the value of any argument.

Of course you can also write

(advice-add
 'hi-lock-write-interactive-patterns :around
 (lambda (f)
   (let ((hi-lock-interactive-patterns
	  (append hi-lock-interactive-patterns
		  hi-lock-file-patterns))))
   (funcall f)))

But if some day `hi-lock-write-interactive-patterns' might become an
optional argument, this advice would raise an wrong number of arguments
error.

> [1]: Another question:
>
> To get patterns from a separate file, I´m using find-file.  This is
> probably not very efficient:  I suspect that the file is actually loaded
> every time, with all what that implies.

No, Emacs doesn't do such things.  If the file is already found in a
buffer, this buffer will just be selected when you `find-file' the same
file again.

If you want to try this approach, you can use `find-file-noselect',
which returns the file's buffer (without selecting it) and assign it to
some variable foo.  Later, you can always just use
(with-current-buffer foo ...).


Regards,

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> > (advice-add
>> >  'hi-lock-write-interactive-patterns :around
>> >  ;; also include `hi-lock-file-patterns'
>> >  (lambda (f &rest args)
>
>> What does (f &rest args) do or mean?
>
>>From the doc of `add-function', this is how :around advices FUNCTION are
> called:
>
>   (lambda (&rest r) (apply FUNCTION OLDFUN r))

I can only guess what that is supposed to mean.  r is undefined, &rest
might be some sort of reference, why the old function is to be applied
is unknown, and where or what the advice is or how to add it doesn´t
show anywhere.  It is also not shown how to put anything actually around
the old function, i. e. how you do something, then call the existing
function, then do some more, which would be the purpose of an around
advice.

> Note that the advice FUNCTION will be called with an additional (the
> first) argument, which will be bound to the original function when the
> advice is called.

The documentation doesn´t say that.  It seems to be supposed to call two
functions, one of them apparently being the existing one, the other one
unknown.

> This way, you have direct access to the original function through that
> binding in your advice, and you can call it with funcall or apply.
> This "mechanism" is the replacement for the old ad-do-it.

Why doesn´t the documentation just say that?

> In my example, FUNCTION is (lambda (f &rest args) ...),

But f and args are undefined.

> which means that f will be bound to OLDFUN, i.e. the original
> definition of `hi-lock-write-interactive-patterns',

But you are not applying something like the documentation would suggest
is needed.

> and args will be bound to r, which will be the list of arguments with
> which `hi-lock-write-interactive-patterns' gets called.

r? So that is something that isn´t defined or documented?  I tried to
look up r, without success other than finding out that it apparently is
neither a variable, nor a function.

>> >    (apply f args)))
>>
>> f args?
>>
>> hi-lock-write-interactive-patterns doesn´t have any arguments.
>
> So, the function will always be called without arguments, which means
> that args will be bound to the empty list - no problem.
>
> So why did I write it that way?  Because it works with any argument
> list.  Instead of specifying the original argument list in the advice,
> I just use  &rest args  and apply the original function to args when I
> don't need to know the value of any argument.
>
> Of course you can also write
>
> (advice-add
>  'hi-lock-write-interactive-patterns :around
>  (lambda (f)
>    (let ((hi-lock-interactive-patterns
> 	  (append hi-lock-interactive-patterns
> 		  hi-lock-file-patterns))))
>    (funcall f)))

How does that go along with the documentation?  The documentation says
"(lambda (&rest r) (apply FUNCTION OLDFUN r))", whatever that means.
You have (f) instead of (&rest r), and "(apply FUNCTION OLDFUN r)" is
missing.  And if f refers to the original function, why not use a before
advice instead?  And I thought "let" limits the scope of the variable to
what is between its brackets.

> But if some day `hi-lock-write-interactive-patterns' might become an
> optional argument, this advice would raise an wrong number of arguments
> error.

What if you want to use one of the arguments?

>> [1]: Another question:
>>
>> To get patterns from a separate file, I´m using find-file.  This is
>> probably not very efficient:  I suspect that the file is actually loaded
>> every time, with all what that implies.
>
> No, Emacs doesn't do such things.  If the file is already found in a
> buffer, this buffer will just be selected when you `find-file' the same
> file again.
>
> If you want to try this approach, you can use `find-file-noselect',
> which returns the file's buffer (without selecting it) and assign it to
> some variable foo.  Later, you can always just use
> (with-current-buffer foo ...).

What when you use find-file-noselect and the file cannot be visited?
The documentation only says it returns the buffer, not what it returns
when it fails.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> > Note that the advice FUNCTION will be called with an additional (the
> > first) argument, which will be bound to the original function when the
> > advice is called.
>
> The documentation doesn´t say that.

It does say it:

  `:around'	(lambda (&rest r) (apply FUNCTION OLDFUN r))
                                             ^^^^^^
> > This way, you have direct access to the original function through that
> > binding in your advice, and you can call it with funcall or apply.
> > This "mechanism" is the replacement for the old ad-do-it.
>
> Why doesn´t the documentation just say that?

Because it's trivial.

> How does that go along with the documentation?  The documentation says
> "(lambda (&rest r) (apply FUNCTION OLDFUN r))", whatever that means.
> You have (f) instead of (&rest r), and "(apply FUNCTION OLDFUN r)" is
> missing.

I think I understand now what you are missing.

  (lambda (&rest r) (apply FUNCTION OLDFUN r))

is _not_ a template of how you would write your advice.  In this line,
FUNCTION means your piece of advice, the function you specify as advice.
The above line describes the semantic of the advised function, i.e., how
the advice will be constructed that will combine the original function
with your advice.

> What if you want to use one of the arguments?

Use an according argument list in FUNCTION, and refer to the arguments in
the function body.

> What when you use find-file-noselect and the file cannot be visited?
> The documentation only says it returns the buffer, not what it returns
> when it fails.

It will raise an error when the file doesn't exist or can't be read, so
you must check that yourself if you need to - see `file-exists-p',
`file-readable-p'.

Michael.

Re: replacing a function with another one

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Because it's trivial.

I have the feeling that this might sound strange - but it's true!  So,
here is some pseudo code that shows, in a simplified manner, what
defining an around advice with `advice-add' does:

(defun my-add-around-advice (fun-to-advice advice)
  (lexical-let ((oldfun (symbol-function fun-to-advice))
		(function advice))
    (fset fun-to-advice
	  (lambda (&rest r) (apply function oldfun r)))))
          
FUN-TO-ADVICE is the function to advice, ADVICE is the piece of advice
you want to add.  Do you recognize the line I cite all the time?  It's
what you get as resulting combined function.

Actually, it's not pseudo code but fully functional.

An example.  Let's define the faculty (a non-recursive version):

(defun my-fac (n) (reduce '* (number-sequence 1 n)))

Suppose we want to make it return the faculty of -n for negative
integers n by adding an around advice (currently it returns 1 for negative
arguments).  With the above simple implementation, you would do it
like that:

(my-add-around-advice
 'my-fac
 (lambda (orig-fun n) (funcall orig-fun (abs n))))

Then, e.g.

  (my-fac -5)

    ==> 120

Of course, you can't remove the advice with my simplified version, etc.

With `advice-add' you would do

(advice-add 'my-fac :around
  (lambda (orig-fun n) (funcall orig-fun (abs n))))

Let's add another around advice that makes `my-fac' print the result in
the echo area.  This time using a named function as advice:

(defun my-fac--print-result-around-advice (orig-fun n)
  "Print result in the echo area."
  (let ((result (funcall orig-fun n)))
    (message "The faculty of %d is %d" n result)
    (sit-for 3)
    result))

(my-add-around-advice 'my-fac #'my-fac--print-result-around-advice)

These are not very useful examples, but hopefully they show a bit how it
works, and one can play with them.  You also see how to deal with
arguments and the return value of the original function.

The other advice types can be implemented similarly.


Regards,

Michael.

Re: replacing a function with another one

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> (defun my-fac--print-result-around-advice (orig-fun n)
>   "Print result in the echo area."
>   (let ((result (funcall orig-fun n)))
>     (message "The faculty of %d is %d" n result)
>     (sit-for 3)
>     result))

BTW, what you also see is that advices in nadvice are just normal,
independently defined functions, and not some obscure expressions.  It's
your function definition, you can name and use the arguments as you
like.  In case of an around advice, your advice FUNCTION will be called
that way:

   (apply function oldfun r)

which means, with the arguments that the original function is called
with, prepended with the old function definition.  It must accept these
arguments.

So, for example, the above defun could also be written as

(defun my-fac--print-result-around-advice
 (orig-fun &rest argument-list-to-pass-over)
  "Print result in the echo area."
  (let ((result (apply orig-fun argument-list-to-pass-over)))
    (message "The faculty of %d is %d" n result)
    (sit-for 3)
    result))



Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Because it's trivial.
>
> I have the feeling that this might sound strange - but it's true!  So,
> here is some pseudo code that shows, in a simplified manner, what
> defining an around advice with `advice-add' does:
>
> (defun my-add-around-advice (fun-to-advice advice)
>   (lexical-let ((oldfun (symbol-function fun-to-advice))
> 		(function advice))
>     (fset fun-to-advice
> 	  (lambda (&rest r) (apply function oldfun r)))))
>           
> FUN-TO-ADVICE is the function to advice, ADVICE is the piece of advice
> you want to add.  Do you recognize the line I cite all the time?  It's
> what you get as resulting combined function.
>
> Actually, it's not pseudo code but fully functional.
>
> An example.  Let's define the faculty (a non-recursive version):
>
> (defun my-fac (n) (reduce '* (number-sequence 1 n)))
>
> Suppose we want to make it return the faculty of -n for negative
> integers n by adding an around advice (currently it returns 1 for negative
> arguments).  With the above simple implementation, you would do it
> like that:
>
> (my-add-around-advice
>  'my-fac
>  (lambda (orig-fun n) (funcall orig-fun (abs n))))
>
> Then, e.g.
>
>   (my-fac -5)
>
>     ==> 120

That sounds like a cryptic way to do the `callinstead' I just suggested
in my previous post :)

> Of course, you can't remove the advice with my simplified version, etc.

And advice-add is lacking what defadvice has with the ability to enable
and to disable the advice.


(callinstead orig-fn new-fn (orig-arg0..orig-argN) :named "removable-callinstead")

(callinstead-remove "removable-callinstead")

(callinstead-add "removable-callinstead")


> With `advice-add' you would do
>
> (advice-add 'my-fac :around
>   (lambda (orig-fun n) (funcall orig-fun (abs n))))
>
> Let's add another around advice that makes `my-fac' print the result in
> the echo area.  This time using a named function as advice:
>
> (defun my-fac--print-result-around-advice (orig-fun n)
>   "Print result in the echo area."
>   (let ((result (funcall orig-fun n)))
>     (message "The faculty of %d is %d" n result)
>     (sit-for 3)
>     result))
>
> (my-add-around-advice 'my-fac #'my-fac--print-result-around-advice)
>
> These are not very useful examples, but hopefully they show a bit how it
> works, and one can play with them.  You also see how to deal with
> arguments and the return value of the original function.
>
> The other advice types can be implemented similarly.

I still don´t understand how it works.  Here´s another example:


lexical-let is a Lisp macro in `cl.el'.

(lexical-let BINDINGS BODY)

Like `let', but lexically scoped.
The main visible difference is that lambdas inside BODY will create
lexical closures as in Common Lisp.


So what is that supposed to mean? `let' keeps driving me insane already
because it requires so many brackets.  Then finally, I do something like


    (let ((end-marker (concat "^" comment-start lsl-hi-lock-patterns-end-marker))
      ((marker-pos (re-search-forward end-marker (point-max) t))))
      (do-stuff))


and it doesn´t work because end-marker is undefined despite I just
defined it :(  So I have


    (let ((end-marker (concat "^" comment-start lsl-hi-lock-patterns-end-marker)))
      (let ((marker-pos (re-search-forward end-marker (point-max) t)))
      (do-something)))


instead ...


And in the end, I´m left with the unanswerable question of how to
intentionally returning something in particular from a function:


(defun lsl-get-patterns-from-file (file)
  "Read hi-lock-mode highlighting-patterns from a file and return
the patterns read."
  (with-current-buffer
      (find-file-noselect file)
    (goto-char (point-min))
    (let ((end-marker (concat "^" comment-start lsl-hi-lock-patterns-end-marker)))
      (let ((marker-pos (re-search-forward end-marker (point-max) t)))
	(when marker-pos
	  (goto-char marker-pos)
	  (previous-line)
	  (end-of-line)
	  (setq marker-pos (point))
	  (goto-char (point-min))
	  (message "reading hi-lock patterns from %s (%d..%d)"
		   (buffer-name)
		   (point-min) marker-pos)
	  (let ((patterns nil))
	    (while (< (point) marker-pos)
	      (setq patterns (append (read (current-buffer)) patterns)))
	    (setq patterns patterns)))))))


I need this function to return `patterns'.  Without the last line, it
seems to return nil because the setq is enclosed in the while.


This is what I made from the example implementation you made.  That
really got me somewhere, and I don´t need to use advices anymore.

Then I found I want to be able to edit the patterns.  Editing them is
easier when there is one pattern per line, so I made it write one per
line and then found that they are suddenly much more difficult to
read.  Finally I got it to read them, just to find out that the function
doesn´t return them.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> So what is that supposed to mean? `let' keeps driving me insane already
> because it requires so many brackets.  Then finally, I do something like
>
>
>     (let ((end-marker (concat "^" comment-start lsl-hi-lock-patterns-end-marker))
>       ((marker-pos (re-search-forward end-marker (point-max) t))))
>       (do-stuff))
>
>
> and it doesn´t work because end-marker is undefined despite I just
> defined it :(  So I have
>
>
>     (let ((end-marker (concat "^" comment-start lsl-hi-lock-patterns-end-marker)))
>       (let ((marker-pos (re-search-forward end-marker (point-max) t)))
>       (do-something)))
>

The functionality you want, and that you reached with the nested `let',
is exactly what `let*' does.  When I was learning LISP, I was also
confused about `let'.  Standard `let' first evaluates all
expressions, then binds the variables, so in works "parallel" - see

  (info "(elisp) Local Variables")

> And in the end, I´m left with the unanswerable question of how to
> intentionally returning something in particular from a function:
>
>
> (defun lsl-get-patterns-from-file (file)
>   "Read hi-lock-mode highlighting-patterns from a file and return
> the patterns read."
>   (with-current-buffer
>       (find-file-noselect file)
>     (goto-char (point-min))
>     (let ((end-marker (concat "^" comment-start lsl-hi-lock-patterns-end-marker)))
>       (let ((marker-pos (re-search-forward end-marker (point-max) t)))
> 	(when marker-pos
> 	  (goto-char marker-pos)
> 	  (previous-line)
> 	  (end-of-line)
> 	  (setq marker-pos (point))
> 	  (goto-char (point-min))
> 	  (message "reading hi-lock patterns from %s (%d..%d)"
> 		   (buffer-name)
> 		   (point-min) marker-pos)
> 	  (let ((patterns nil))
> 	    (while (< (point) marker-pos)
> 	      (setq patterns (append (read (current-buffer)) patterns)))
> 	    (setq patterns patterns)))))))
>
>
> I need this function to return `patterns'.  Without the last line, it
> seems to return nil because the setq is enclosed in the while.

`let' returns the value returned by the last body expression (in
particular, that isn't necessarily the value of the variable `patterns'
in your example!).  And `while' always returns nil (strictly speaking,
the return value isn't documented, so don't rely on it at all).  So this
is what you want:

 	  (let ((patterns nil))
 	    (while (< (point) marker-pos)
 	      (setq patterns (append (read (current-buffer)) patterns)))
 	    patterns)



Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> and it doesn´t work because end-marker is undefined despite I just
>> defined it :(  So I have
>>
>>
>>     (let ((end-marker (concat "^" comment-start lsl-hi-lock-patterns-end-marker)))
>>       (let ((marker-pos (re-search-forward end-marker (point-max) t)))
>>       (do-something)))
>>
>
> The functionality you want, and that you reached with the nested `let',
> is exactly what `let*' does.  When I was learning LISP, I was also
> confused about `let'.  Standard `let' first evaluates all
> expressions, then binds the variables, so in works "parallel" - see

Ah!  Finally I understand what the difference is!  What is the point of
having two variations of `let'?


> [...]
>> 	  (let ((patterns nil))
>> 	    (while (< (point) marker-pos)
>> 	      (setq patterns (append (read (current-buffer)) patterns)))
>> 	    (setq patterns patterns)))))))
>>
>>
>> I need this function to return `patterns'.  Without the last line, it
>> seems to return nil because the setq is enclosed in the while.
>
> `let' returns the value returned by the last body expression (in
> particular, that isn't necessarily the value of the variable `patterns'
> in your example!).  And `while' always returns nil (strictly speaking,
> the return value isn't documented, so don't rely on it at all).  So this
> is what you want:
>
>  	  (let ((patterns nil))
>  	    (while (< (point) marker-pos)
>  	      (setq patterns (append (read (current-buffer)) patterns)))
>  	    patterns)

Thank you --- I only found that out yesterday.  It makes sense when you
think that a symbol stands for itself --- which is probably not correct,
but I just think of it as that the variable stands for itself and can be
returned like that.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> Ah!  Finally I understand what the difference is!  What is the point of
> having two variations of `let'?

Both are useful.  The standard `let' works similar to how arguments of a
lambda expression are bound.  If the expressions expr1, expr2, ... in a
`let' form

  (let ((v1 expr1)
        (v2 expr2)
      ...)
  body)

are side-effect free, the bindings of the `let' are interchangeable and
independent from each other, which makes code easier to read in general.

OTOH, `let*' provides the possibility to refer to bindings made earlier,
which is often what you want.
But to the cost that every binding may depend on prior bindings in the
same `let*', which makes it harder to read.

BTW, there is also a parallel setting operator named `psetq' or
`cl-psetq' respectively.  For example, to interchange the values bound
to the variables a, b, you can do

   (psetq a b b a)


Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> > Note that the advice FUNCTION will be called with an additional (the
>> > first) argument, which will be bound to the original function when the
>> > advice is called.
>>
>> The documentation doesn´t say that.
>
> It does say it:
>
>   `:around'	(lambda (&rest r) (apply FUNCTION OLDFUN r))
>                                              ^^^^^^
>> > This way, you have direct access to the original function through that
>> > binding in your advice, and you can call it with funcall or apply.
>> > This "mechanism" is the replacement for the old ad-do-it.
>>
>> Why doesn´t the documentation just say that?
>
> Because it's trivial.

It is complicated and cryptic.  That line doesn´t tell me anything, it
only confuses me.  Why and how would I apply and old function with an
anonymous function that appears to be a named one?  And r is still
undefined.

>> How does that go along with the documentation?  The documentation says
>> "(lambda (&rest r) (apply FUNCTION OLDFUN r))", whatever that means.
>> You have (f) instead of (&rest r), and "(apply FUNCTION OLDFUN r)" is
>> missing.
>
> I think I understand now what you are missing.
>
>   (lambda (&rest r) (apply FUNCTION OLDFUN r))
>
> is _not_ a template of how you would write your advice.  In this line,
> FUNCTION means your piece of advice, the function you specify as advice.
> The above line describes the semantic of the advised function, i.e., how
> the advice will be constructed that will combine the original function
> with your advice.

Why can´t I just specify that my own function should be called instead
of the existing one and then that I can call the original function from
within my function?  After all, that is what this kind of advice is
supposed to be for.  Something like:


(defun replacement-fn (original-fn-arg0 original-fn-arg1)
  (foobar)
  (original-fn original-fn-arg0 original-fn-arg1)
  (barfoo))

(callinstead original-fn replacement-fn)


That would be clear and simple, assuming that calling the original
function from within a function that is defined to be called instead of
it always automatically calls the original function.  Otherwise, use
something like (calloriginal (original-fn original-fn-arg0
original-fn-arg1)) --- that´s probably clearer anyway.

You could also have `callbefore', `callafter', etc.  Maybe it´s even
possible to implement this, using add-advice.

Perhaps for some things you´d still have to fall back to using
add-advice directly.  For 99% of the cases, it would be great.

You might take it a step further and provide something to check whether
the original-fn has changed, like a hash of the version the callinstead
was written for.  Put the hash into the code like


(callinstead-hash original-fn "<hash-of-original-fn>")


and when your code is loaded or before it´s evaluated, the hash is
verified and you get a warning when the original-fn has changed.  Add a
flag or something that optionally makes using a hash mandatory.

>> What if you want to use one of the arguments?
>
> Use an according argument list in FUNCTION, and refer to the arguments in
> the function body.

Like how?

>> What when you use find-file-noselect and the file cannot be visited?
>> The documentation only says it returns the buffer, not what it returns
>> when it fails.
>
> It will raise an error when the file doesn't exist or can't be read, so
> you must check that yourself if you need to - see `file-exists-p',
> `file-readable-p'.

ok


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Stefan Monnier]

> That line doesn´t tell me anything, it only confuses me.

Probably because you don't understand (lambda ...) yet.

> (defun replacement-fn (original-fn-arg0 original-fn-arg1)
>   (foobar)
>   (original-fn original-fn-arg0 original-fn-arg1)
>   (barfoo))

> (callinstead original-fn replacement-fn)

That's pretty much exactly what advice-add does for :around, except that
the original-fn is provided as an additional argument, so you have to write:

   (defun replacement-fn (orig-fun original-fn-arg0 original-fn-arg1)
     (foobar)
     (funcall orig-fun original-fn-arg0 original-fn-arg1)
     (barfoo))
  
   (advice-add 'original-fn :around 'replacement-fn)

> That would be clear and simple, assuming that calling the original
> function from within a function that is defined to be called instead of
> it always automatically calls the original function.

That's one of the reasons why the original function is passed as an
extra argument.  This way you can choose to either call (original-fn ...)
which will go through the advice recursively, or to call (funcall
orig-fun ...) which will call the unadvised version.

Magically making `original-fn' detect when it's called from its own
advice and skip the advice in this case would be not only less general,
but also pretty ugly to implement and unreliable.

> You could also have `callbefore', `callafter', etc.  Maybe it´s even
> possible to implement this, using add-advice.

Yup, just use :after or :before (in which case your advice won't receive
the `orig-fun' argument).


        Stefan

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> > Of course you can also write
> >
> > (advice-add
> >  'hi-lock-write-interactive-patterns :around
> >  (lambda (f)
> >    (let ((hi-lock-interactive-patterns
> > 	  (append hi-lock-interactive-patterns
> > 		  hi-lock-file-patterns))))
> >    (funcall f)))
>
> And if f refers to the original function, why not use a before advice
> instead?  And I thought "let" limits the scope of the variable to what
> is between its brackets.

Good catch!  Yes, sorry, the parens were wrong, I meant this:

(advice-add
 'hi-lock-write-interactive-patterns :around
 (lambda (f)
   (let ((hi-lock-interactive-patterns
	  (append hi-lock-interactive-patterns
		  hi-lock-file-patterns)))
     (funcall f))))


Thanks,

Michael.

Re: replacing a function with another one

[Stefan Monnier]

> (defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
>   ...
>   (ad-deactivate 'hi-lock-set-file-patterns)
[...]
> (defun my-test ()
>   ...
>   (ad-activate 'hi-lock-set-file-patterns)

I recommend you don't use ad-activate and ad-deactivate (among other
reasons because they activate/deactivate all advices applied to the
function rather than only the one you know about; but also because they
hide the existence of an advice).

Instead, use an auxiliary variable, e.g.

  (defvar my-set-patterns-in-original-buffer nil)
  (defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
    (when my-set-patterns-in-original-buffer
      (setq my-set-patterns-in-original-buffer nil)
      ...))
  (defun my-test ()
    (setq my-set-patterns-in-original-buffer t)
    ...)
or
  (defvar my-set-patterns-in-original-buffer nil)
  (defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
    (when my-set-patterns-in-original-buffer
      ...))
  (defun my-test ()
    (let ((my-set-patterns-in-original-buffer t))
      ...))

BTW, I recommend you then merge my-set-patterns-in-original-buffer and
original-buffer, so the advice is always active but only does something
if original-buffer is non-nil.  Also, use with-current-buffer rather
than pop-to-buffer (pop-to-buffer has *many* side-effects, so using it
in an advice is a recipe for disaster):

  (defvar my-set-patterns-original-buffer nil)
  (defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
    (when my-set-patterns-original-buffer
      (with-current-buffer my-set-patterns-original-buffer
        (hi-lock-set-file-patterns (ad-get-arg 0)))))
  (defun my-test ()
    (save-excursion
      (save-restriction
        (widen)
        (goto-char (point-min))
        (when (re-search-forward "^// ext-fontify-defs: " nil t)
	  (message "found marker")
	  (let ((filename (thing-at-point 'filename)))
	    (message "filename at %d: %s/%s for buffer %s" (point) filename (thing-at-point 'filename) (buffer-name))
	    (when filename
	      (when (file-exists-p filename)
	      (let ((my-set-patterns-original-buffer (current-buffer)))
                ;; FIXME: usually from Lisp we should call find-file-noselect,
                ;; but I don't know what's the intention here.
	        (find-file filename)
	        (hi-lock-mode 1)))))))))

-- Stefan

Re: replacing a function with another one

[lee]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> (defadvice hi-lock-set-file-patterns (after my-test-obtain-patterns)
>>   ...
>>   (ad-deactivate 'hi-lock-set-file-patterns)
> [...]
>> (defun my-test ()
>>   ...
>>   (ad-activate 'hi-lock-set-file-patterns)
>
> I recommend you don't use ad-activate and ad-deactivate (among other
> reasons because they activate/deactivate all advices applied to the
> function rather than only the one you know about; but also because they
> hide the existence of an advice).
>
> Instead, use an auxiliary variable, e.g.

Hm, I have experimented with variables and found that things don´t work
when the variables are local.  But then, I can´t get add-advice to work,
not even with global variables.  It´s too complicated, and the
documentation is not understandable at all.

It also seems that it is not possible at all to make a mode on top of
hi-lock-mode which does what I want.  Variables have to be local per
buffer, and with that, it´s not possible to read patterns from different
buffers because the variables change because buffers must be switched to
read the patterns.

Perhaps I can fork hi-lock.el and somehow make it work with multiple
buffers, but I doubt it.

> BTW, I recommend you then merge my-set-patterns-in-original-buffer and
> original-buffer, so the advice is always active but only does something
> if original-buffer is non-nil.

That doesn´t work with global variables because for one buffer, the
original-buffer may be nil when patterns are not read from another
buffer.

> Also, use with-current-buffer rather than pop-to-buffer (pop-to-buffer
> has *many* side-effects, so using it in an advice is a recipe for
> disaster):

ok

>                 ;; FIXME: usually from Lisp we should call find-file-noselect,
>                 ;; but I don't know what's the intention here.
> 	        (find-file filename)

The intention was to load the file so that hi-lock-mode reads the
patterns from it.  I simply didn´t know what else to use.  It would be
nice if things like this would be mentioned in the documentation ...


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> > Instead, use an auxiliary variable, e.g.
>
> Hm, I have experimented with variables and found that things don´t work
> when the variables are local.  But then, I can´t get add-advice to work,
> not even with global variables.  It´s too complicated, and the
> documentation is not understandable at all.
>
> It also seems that it is not possible at all to make a mode on top of
> hi-lock-mode which does what I want.  Variables have to be local per
> buffer, and with that, it´s not possible to read patterns from different
> buffers because the variables change because buffers must be switched to
> read the patterns.

Please don't get confused.  Use global auxiliary variables for things
that reflect buffer independent states.  And you can always get the
buffer local binding of a variable foo in any buffer b with

  (with-current-buffer b foo)

There is no limitation from the Emacs' side.

Mmh, ok, let's start from the beginning and simple.  Let's add some
mechanism that allows to specify additional, directory local hi-lock
patterns.  That should be simple as this:

    (defvar hi-lock-dir-patterns nil)  ;; use as dir local variable
    
    (advice-add
     'hi-lock-find-patterns :after
     (lambda (&rest _)
       (hi-lock-set-file-patterns
        (append hi-lock-file-patterns
                hi-lock-dir-patterns))))

The advice just merges in the additional patterns.  Note that using
directory local variables has the advantage that me must not explicitly
look up the patterns, because the directory local variable is
automatically bound when the file is opened.

This should be all to do as long as you don't want to automatically save
additional patterns - you have to add them yourself in the .dir-locals
file.

If you want such automatic saving, we would just have to substitute the
dir local variable with a mechanism that loads and saves explicitly.  Is
this what you want?


Regards,

Michael.

Re: replacing a function with another one

[Michael Heerdegen]

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> If you want such automatic saving, we would just have to substitute the
> dir local variable with a mechanism that loads and saves explicitly.  Is
> this what you want?

I've created an example implementation:


[-- Attachment #2: hi-lock-lee.el --]
[-- Type: application/emacs-lisp, Size: 984 bytes --]

[-- Attachment #3: Type: text/plain, Size: 489 bytes --]


You can specify the pattern save file in the local variables
section at the end of a file, but before a hi-lock mode specification,
like here:

Local Variables:
hi-lock-patterns-file: "patterns.txt"
mode: hi-lock
End:


Several files can share one pattern file.  The example completely
substitutes file local patterns, if you want to use them at the same
time, it will become more complicated.  If you don't bind
`hi-lock-patterns-file', the behavior is like before.


Regards,

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> You can specify the pattern save file in the local variables
> section at the end of a file, but before a hi-lock mode specification,
> like here:
>
> Local Variables:
> hi-lock-patterns-file: "patterns.txt"
> mode: hi-lock
> End:

Apparently this doesn´t work because hi-lock-mode is called before the
variable is getting a value assigned.  See `hack-one-local-variable´ in
files.el --- IIUC, it calls the mode before it makes the variable.  It´s
hilarious ...

Since now I learned that a function is called for this, I can experiment
with a function defined in the mode I´m working on to read the patterns
and to set them.  This should be possible without using an advice.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:


> > You can specify the pattern save file in the local variables
> > section at the end of a file, but before a hi-lock mode specification,
> > like here:
> >
> > Local Variables:
> > hi-lock-patterns-file: "patterns.txt"
> > mode: hi-lock
> > End:
>
> Apparently this doesn´t work because hi-lock-mode is called before the
> variable is getting a value assigned.  See `hack-one-local-variable´ in
> files.el --- IIUC, it calls the mode before it makes the variable.  It´s
> hilarious ...

No, that works well here, I've tested in detail what I had posted here,
and it worked well.  AFAIK, local variable specifications are processed
in the order as they appear textually in the file - I see that in the
code.  Maybe this is not the case in your Emacs version ... or you
didn't use it in the intended way.

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>
>> > You can specify the pattern save file in the local variables
>> > section at the end of a file, but before a hi-lock mode specification,
>> > like here:
>> >
>> > Local Variables:
>> > hi-lock-patterns-file: "patterns.txt"
>> > mode: hi-lock
>> > End:
>>
>> Apparently this doesn´t work because hi-lock-mode is called before the
>> variable is getting a value assigned.  See `hack-one-local-variable´ in
>> files.el --- IIUC, it calls the mode before it makes the variable.  It´s
>> hilarious ...
>
> No, that works well here, I've tested in detail what I had posted here,
> and it worked well.  AFAIK, local variable specifications are processed
> in the order as they appear textually in the file - I see that in the
> code.

What I´m referring to is not in which order they are created compared to
where they appear in the text.  When a specification of a variable is
encountered, what is in "mode:" will be called as a function if it ends
in "-mode".  Calling such function always seems to happen before the
variable is created.

> Maybe this is not the case in your Emacs version ... or you
> didn't use it in the intended way.

Yes, I´m using it in a different way.  The mode I´m doing this with is
not hi-lock-mode but lsl-mode.  lsl-mode requires hi-lock-mode because
it´s using it.

With "mode: hi-lock", not much did happen and it somehow wasn´t right.
So I put "mode: lsl-mode" instead and was surprised to get an error
message saying that lsl-mode-mode is a void function.  So I found out
about this sort of function calling.

It´s a cool feature, though.  So I made a function lsl-mode-mode, yet
the variable was only created after I would need it.  Consequently, I
abandoned this approach and went back to just put a line into the file
which is searched for and the file name is taken from.

When it´s found, read the patterns from that file and use
hi-lock-set-file-patterns to set them.  That works fine without any
advices.

Now when lsl-mode is enabled, just do that (i. e. search, read, apply).
It could even be done for several files that include patterns by doing
it over and over again until no more files to include are found.

It´s working; the only problem I have atm is how to intentionally return
a particular value from a function ...

Once it´s a bit nicer, I want to make it a mode on its own.  I think
it´s too useful to leave it limited to lsl-mode.  I guess I can make it
a mode derived from hi-lock mode; it simply adds some functionality to
hi-lock.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Stefan Monnier]

> encountered, what is in "mode:" will be called as a function if it ends
> in "-mode".  Calling such function always seems to happen before the
> variable is created.

"mode:" is for *major* modes, i.e. not for hi-lock-mode.
You want to use "eval: (hi-lock-mode)" instead.


        Stefan

Re: replacing a function with another one

[lee]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> encountered, what is in "mode:" will be called as a function if it ends
>> in "-mode".  Calling such function always seems to happen before the
>> variable is created.
>
> "mode:" is for *major* modes, i.e. not for hi-lock-mode.
> You want to use "eval: (hi-lock-mode)" instead.

Does this create the variable before the mode is enabled?  If so, it
would save me searching for the name of the file to read the patterns
from.


What if someone edits the buffer and changes the value of this variable?
Will it be updated automatically?

Currently, the functions I have search for the file name in the buffer
every time it´s needed because it could have been modified.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> > "mode:" is for *major* modes, i.e. not for hi-lock-mode.
> > You want to use "eval: (hi-lock-mode)" instead.
>
> Does this create the variable before the mode is enabled?

AFAIK, yes, as long as the variable is mentioned before the
"eval:(hi-lock-mode)".

> What if someone edits the buffer and changes the value of this
> variable?  Will it be updated automatically?

No, you would have to do this explicitly.

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> > "mode:" is for *major* modes, i.e. not for hi-lock-mode.
>> > You want to use "eval: (hi-lock-mode)" instead.
>>
>> Does this create the variable before the mode is enabled?
>
> AFAIK, yes, as long as the variable is mentioned before the
> "eval:(hi-lock-mode)".

I tried that, and it didn`t work.

>> What if someone edits the buffer and changes the value of this
>> variable?  Will it be updated automatically?
>
> No, you would have to do this explicitly.

I have made it so that by default, the variable is read from the
buffer.  So either way, it is being set before it`s needed, unless you
customise it differently.

That also has the advantage that when its value changes, it can  be
re-read from the buffer.

I think I`ll give it a try to add this to hi-lock-mode --- it would make
sense to have it as part of it ...


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> > Instead, use an auxiliary variable, e.g.
>>
>> Hm, I have experimented with variables and found that things don´t work
>> when the variables are local.  But then, I can´t get add-advice to work,
>> not even with global variables.  It´s too complicated, and the
>> documentation is not understandable at all.
>>
>> It also seems that it is not possible at all to make a mode on top of
>> hi-lock-mode which does what I want.  Variables have to be local per
>> buffer, and with that, it´s not possible to read patterns from different
>> buffers because the variables change because buffers must be switched to
>> read the patterns.
>
> Please don't get confused.  Use global auxiliary variables for things
> that reflect buffer independent states.

But these things are not global.  There is a combination of buffer A and
buffer B, buffer C, ..., buffer N.  The buffers B..N can all contain
patterns that need to be read and used to for highlighting with buffer
A.

To keep it simple, I have assumed that there is only buffer A and buffer
B.  Support for multiple buffers might be added later.

Now for buffer A, it must be remembered that its patters are in buffer
B.  That goes for multiple combinations of buffers A and B.

When the variables used to remember this are global, there can only be a
single combination of buffers.

I have experimented with defvar-local:


(defvar-local test nil)

(defun foo
(setq test (current-buffer))
(message "test-1 %s" (buffer-name test))
(find-file "some.file")
(message "test-2 %s" (buffer-name test)))


When you call foo, the value of test will change according to the
current buffer.  So you cannot use a local variable.  Use a global one
and it doesn´t change, but then you don´t know anymore from which buffer
patterns were used for which buffer when you do it with multiple files.

> And you can always get the buffer local binding of a variable foo in
> any buffer b with
>
>   (with-current-buffer b foo)
>
> There is no limitation from the Emacs' side.

But then I don´t have the variables from the buffer I´m doing this from.

> Mmh, ok, let's start from the beginning and simple.  Let's add some
> mechanism that allows to specify additional, directory local hi-lock
> patterns.  That should be simple as this:
>
>     (defvar hi-lock-dir-patterns nil)  ;; use as dir local variable
>     
>     (advice-add
>      'hi-lock-find-patterns :after
>      (lambda (&rest _)
>        (hi-lock-set-file-patterns
>         (append hi-lock-file-patterns
>                 hi-lock-dir-patterns))))
>

I can´t get advice-add to work.  The documentation doesn´t explain it,
and google doesn´t find any examples.  It either doesn´t work, or I´m
getting weird error messages that don´t tell me anything.  Code that
works fine when used in a function suddenly gives an error message that
says that setq is an invalid function.  Remove the (setq something
tosomething) and suddenly all the rest of the function is invalid for no
reason.

> The advice just merges in the additional patterns.

Well, it can only be used :before or :after.  Using it :before didn´t
work because I had to switch buffers in the prepended part, and
apparently after that part was completed, the buffer was switched back.

> Note that using directory local variables has the advantage that me
> must not explicitly look up the patterns, because the directory local
> variable is automatically bound when the file is opened.

?

> This should be all to do as long as you don't want to automatically save
> additional patterns

That is one of the next steps.  With add-advice, It´s not working at all
atm.

> - you have to add them yourself in the .dir-locals
> file.
>
> If you want such automatic saving, we would just have to substitute the
> dir local variable with a mechanism that loads and saves explicitly.  Is
> this what you want?

I simply want to store the patterns in one (and at some time multiple)
different buffers rather than in the buffer they are applied to.  I want
that to work transparently, for saving, loading and keeping the patterns
up to date (i. e. when something that used to be highlighted is
completely removed, the pattern for it needs to be removed, too),
without duplicates.  To assign a file A with patterns to another file B, I
just want to put a line into file B so that the patterns in file A are
used to highlight file B.  File A can than be used with many files B.

Once that works, the files A could have multiple lines in them to use
patterns from multiple files B.

Files B do not need to have patterns in themselves, though it would be
nice if they could.


Sigh ... Any idea why the following does not work:


(defvar ext-hi-lock-original-buffer nil
  "Remember the buffer for which hi-lock highlighting-patterns
  that are being read from another buffer are to be applied to
  --- that is the original buffer.")


(defvar ext-hi-lock-patterns-buffer nil)


(defvar ext-hi-lock-disable-all nil)


(defun my-test-unless ()
  (interactive)
  (unless ext-hi-lock-disable-all
    (message "enabled")))


;;
;; functions modifying functions in hi-lock.el
;;

(defun ext-hi-lock-find-other-patterns ()
  "See if there is a line in the current buffer specifying that
hi-lock highlighting-patterns should be read from another
file. When the line is found, visit the other file and read the
patterns from its buffer, then apply them to the current buffer."
  (unless ext-hi-lock-disable-all
    ;; (when (not ext-hi-lock-original-buffer)
    (setq ext-hi-lock-original-buffer (current-buffer))
    (save-excursion
      (save-restriction
	(widen)
	(goto-char (point-min))
	(when (re-search-forward "^// ext-fontify-defs: " nil t)
	  (let ((filename (thing-at-point 'filename)))
	    (when filename
	      (when (file-exists-p filename)
		(message "start: original-buffer: %s, current-b: %s"
			 (buffer-name ext-hi-lock-original-buffer)
			 (buffer-name (current-buffer)))
		(setq ext-hi-lock-patterns-buffer (find-file-noselect filename))
		(message "found file: original-buffer: %s, current-b: %s"
			 (buffer-name ext-hi-lock-original-buffer)
			 (buffer-name (current-buffer)))
		(when ext-hi-lock-patterns-buffer
		  (message "get patterns from %s"
			   (buffer-name ext-hi-lock-patterns-buffer))
		  (set-buffer ext-hi-lock-patterns-buffer)
		  (setq ext-hi-lock-disable-all t)
		  (set-auto-mode-0 'hi-lock-mode t)
		  (setq ext-hi-lock-disable-all nil)
		  (message "buffer set: original-buffer: %s, current-b: %s"
			   (buffer-name ext-hi-lock-original-buffer)
			   (buffer-name (current-buffer)))
		  ;; search the whole buffer for patterns
		  (setq hi-lock-file-patterns-range (point-max)))))))))))


(advice-add
 'hi-lock-find-patterns :before
 'ext-hi-lock-find-other-patterns)

(advice-add
 'hi-lock-set-file-patterns :before
 (lambda (patterns)
   (my-test-unless)
   (unless ext-hi-lock-disable-all
     (message "set on buffer: %s"
	      (buffer-name (current-buffer)))
     (set-buffer ext-hi-lock-original-buffer)
     (message "set now buffer: %s --- should be buffer %s"
	      (buffer-name (current-buffer))
	      (buffer-name ext-hi-lock-original-buffer))
     (setq ext-hi-lock-disable-all t)
     (hi-lock-set-file-patterns patterns)
     (setq ext-hi-lock-disable-all nil)
     ))
 '((name . ext-hi-lock-switch-to-original-buffer)))


(defun ext-hi-lock-message (unused)
  (message "patterns probably from %s for %s"
	   ext-hi-lock-patterns-buffer
	   ext-hi-lock-original-buffer))

(setq hi-lock-file-patterns-policy 'ext-hi-lock-message)


It just doesn´t apply the patterns.  For unknown reasons,
hi-lock-set-file-patterns is not called when the original buffer has a
line in it to pull in patterns from another buffer.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> Sigh ... Any idea why the following does not work:

Your code does something you definitely shouldn't do: your advices make
`hi-lock-set-file-patterns' and `hi-lock-find-patterns' switch to a
different buffer (persistently!).  When you need the context of another
buffer, use `with-current-buffer' to switch to it temporarily.

Maybe the following illustration may help:

(defvar-local foo)

Suppose we have two buffers buffer-a and buffer-b.  buffer-a is current,
and we want to set `foo's binding in buffer-a to `foo's value in
buffer-b (I think this is what you try to do, more or less).  This is
how you can do it

--8<---------------cut here---------------start------------->8---
(let (helper)    ; temporary variable (not buffer local)
  
  (with-current-buffer buffer-b
    ;; set it to `foo's binding in buffer-b
    (setq helper foo))
  
  ;;back in buffer-a
  (setq foo helper))
--8<---------------cut here---------------end--------------->8---


BTW, did you have a look at my example?  I think it's quite close to
what you want to achieve.

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> Sigh ... Any idea why the following does not work:
>
> Your code does something you definitely shouldn't do: your advices make
> `hi-lock-set-file-patterns' and `hi-lock-find-patterns' switch to a
> different buffer (persistently!).  When you need the context of another
> buffer, use `with-current-buffer' to switch to it temporarily.

The idea is very simple:  When patterns are to be read from another
buffer, switch to that buffer before they are being read.  Before
applying them, switch back to the original buffer.

I can´t do that with `with-current-buffer´ because two different functions
need to be advised to do it, and I can´t get add-advice to work with
:around.

Why shouldn´t I switch buffers?  They are switched back anyway.

In any case, why doesn´t it work?  Is there a way to see what´s going
on, like some debugger that allows me to run it line by line?  I really
want to know why it doesn´t work.

> Maybe the following illustration may help:
>
> (defvar-local foo)
>
> Suppose we have two buffers buffer-a and buffer-b.  buffer-a is current,
> and we want to set `foo's binding in buffer-a to `foo's value in
> buffer-b (I think this is what you try to do, more or less).  This is
> how you can do it
>
> (let (helper)    ; temporary variable (not buffer local)
>   
>   (with-current-buffer buffer-b
>     ;; set it to `foo's binding in buffer-b
>     (setq helper foo))
>   
>   ;;back in buffer-a
>   (setq foo helper))

That´s why it might be better to make a fork of hi-lock.el.  I could
modify the functions directly and add other ones I´d need.  Or maybe I
should just re-define some of the functions.

In any case, when hi-lock.el changes, what I´m doing may not work
anymore.  So what difference does it make?

> BTW, did you have a look at my example?  I think it's quite close to
> what you want to achieve.

Yes, it looks very interesting :)

One thing I haven´t been able to figure out is how hi-lock.el decides
what the lines it writes to the buffer are prepended with.  In one
buffer, it puts "// Hi-lock ...", in another one, it´s "# Hi-lock ...".
Then it searches patterns with '"\\<" hi-lock-file-patterns-prefix ":"',
and apparently it will not find patterns prepended with "//" when it
figures that they should be prepended with "#".

The regexp for a comment starter is "\s<"[1], and it is not used in the
search.  So why aren´t patterns found when an unexpected comment starter
is used with the patterns written to a buffer?


[1]: http://www.emacswiki.org/emacs/RegularExpression


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> The idea is very simple:  When patterns are to be read from another
> buffer, switch to that buffer before they are being read.  Before
> applying them, switch back to the original buffer.
>
> I can´t do that with `with-current-buffer´ because two different functions
> need to be advised to do it, and I can´t get add-advice to work with
> :around.
>
> Why shouldn´t I switch buffers?  They are switched back anyway.

But what the code that's being run in the meantime, between your
advices?  Then, the wrong buffer is current.  That can't work.

> One thing I haven´t been able to figure out is how hi-lock.el decides
> what the lines it writes to the buffer are prepended with.  In one
> buffer, it puts "// Hi-lock ...", in another one, it´s "# Hi-lock ...".
> Then it searches patterns with '"\\<" hi-lock-file-patterns-prefix ":"',
> and apparently it will not find patterns prepended with "//" when it
> figures that they should be prepended with "#".

When writing the specification to he buffer, it calls `comment-region',
which DTRT in any mode.

`hi-lock-find-patterns' indeed doesn't seem to search only inside
comments, it just searches the whole buffer for the regexp

   (concat "\\<" hi-lock-file-patterns-prefix ":")


Regards,

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>>
>> Why shouldn´t I switch buffers?  They are switched back anyway.
>
> But what the code that's being run in the meantime, between your
> advices?  Then, the wrong buffer is current.  That can't work.

Hm, that would depend on what is being run.  I see what you mean: Since
everything can be modified, there is no way to know if something runs in
between and what that might be.

>> One thing I haven´t been able to figure out is how hi-lock.el decides
>> what the lines it writes to the buffer are prepended with.  In one
>> buffer, it puts "// Hi-lock ...", in another one, it´s "# Hi-lock ...".
>> Then it searches patterns with '"\\<" hi-lock-file-patterns-prefix ":"',
>> and apparently it will not find patterns prepended with "//" when it
>> figures that they should be prepended with "#".
>
> When writing the specification to he buffer, it calls `comment-region',
> which DTRT in any mode.
>
> `hi-lock-find-patterns' indeed doesn't seem to search only inside
> comments, it just searches the whole buffer for the regexp
>
>    (concat "\\<" hi-lock-file-patterns-prefix ":")

Yes, that´s what I thought.  I usually search for what I have written
when doing things like that.

Anyway, all the documentation I found says that "\s<" in a regex is
supposed to match a comment starter.  That doesn´t seem to work at all,
and I ended up with


(concat "^" comment-start lsl-hi-lock-patterns-end-marker).


where


(concat "^\\s<" lsl-hi-lock-patterns-end-marker)


supposedly works but doesn´t.  Is that a bug?


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> (concat "^\\s<" lsl-hi-lock-patterns-end-marker)

How did you search for it?

   (search-forward-regexp "^\\s<")

works well for me.

Michael.

Re: replacing a function with another one

[Stefan Monnier]

> How did you search for it?
>    (search-forward-regexp "^\\s<")
> works well for me.

Maybe he's confused by the fact that \s< won't match multi-char
comment starters.


        Stefan

Re: replacing a function with another one

[lee]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> How did you search for it?
>>    (search-forward-regexp "^\\s<")
>> works well for me.
>
> Maybe he's confused by the fact that \s< won't match multi-char
> comment starters.

That`s what I tried to use it with.  The documentation says it matches a
comment start and nowhere mentions that it doesn`t work for starters
like '//'.

So I used `comment-start', which may be nil by the time it`s used ...


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Stefan]

> That`s what I tried to use it with.  The documentation says it matches a
> comment start and nowhere mentions that it doesn`t work for starters
> like '//'.

Where does it say "it matches a comment start"?  That'd be a bug we need
to fix.

I suspect it doesn't actually say that, but that you did not notice the
subtle difference in wording.


        Stefan

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> I can´t get advice-add to work.  The documentation doesn´t explain it,

That's not true.  Technically you find everything you need to know in
the documentation.  The problem is that it does say nothing at all about
usage in practice, no examples, etc.  It's absolutely not sufficient for
beginners.  That's indeed a problem - there is already a bug report
about it, btw.

> and google doesn´t find any examples.  It either doesn´t work, or I´m
> getting weird error messages that don´t tell me anything.  Code that
> works fine when used in a function suddenly gives an error message that
> says that setq is an invalid function.  Remove the (setq something
> tosomething) and suddenly all the rest of the function is invalid for no
> reason.

If you feel better with the "old" advice.el, just use this.  It will
probably be still supported in many years, just because many third party
packages use it.

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> I can´t get advice-add to work.  The documentation doesn´t explain it,
>
> That's not true.  Technically you find everything you need to know in
> the documentation.

"Food is edible.  Technically, that´s all you need to know."

That doesn´t explain anything about food.  It´s like saying "1 is (1 + 1
is 2)", which doesn´t explain anything, either.

> The problem is that it does say nothing at all about usage in
> practice, no examples, etc.  It's absolutely not sufficient for
> beginners.  That's indeed a problem - there is already a bug report
> about it, btw.

Maybe the problem is that add-advice is too complicated and convoluted.
I learned about defadvice in like no time, it´s clear and simple ---
while advice-add is anything but.

> If you feel better with the "old" advice.el, just use this.  It will
> probably be still supported in many years, just because many third party
> packages use it.

And ppl might continue to use defadvice because it´s clear and simple.
I don´t feel better with it, though.  It has been replaced, and the
replacement seems to lead to better code, so I´d rather use that.


But thinking of it ...  When I´m using advices, they go for all the
advised functions.  Someone might want to use hi-lock mode the same way
they used to and only want the modified behaviour with particular
buffers.

Considering all that, wouldn´t it be much better to create my own mode?


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> Maybe the problem is that add-advice is too complicated and convoluted.
> I learned about defadvice in like no time, it´s clear and simple ---
> while advice-add is anything but.

On the contrary.  nadvice is as simple as it could be.  To understand
how an :around advice works, it takes one line

  (lambda (&rest r) (apply FUNCTION OLDFUN r))

That's a non-ambiguous specification from which you can clearly derive
how the advice FUNCTION will be used.  It can't be simpler.  The problem
is that many people are not so used to lambda expressions and parameter
substitution and so they don't understand it.  If you take a pencil and
a sheet of paper and evaluate an example for yourself by hand, step by
step, you'll understand how it works.  And no, this is not missing in
the documentation.  The Emacs documentation is not an introduction into
lambda calculus and writing LISP code.  Once you have really understood
how evaluation of expressions in LISP works, you'll understand the above
definition.

> Considering all that, wouldn´t it be much better to create my own
> mode?

I don't think this is appropriate, because what you want can be done in
few lines without making much assumptions about the original package.

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> Maybe the problem is that add-advice is too complicated and convoluted.
>> I learned about defadvice in like no time, it´s clear and simple ---
>> while advice-add is anything but.
>
> On the contrary.  nadvice is as simple as it could be.  To understand
> how an :around advice works, it takes one line
>
>   (lambda (&rest r) (apply FUNCTION OLDFUN r))
>
> That's a non-ambiguous specification from which you can clearly derive
> how the advice FUNCTION will be used.  It can't be simpler.  The problem
> is that many people are not so used to lambda expressions and parameter
> substitution and so they don't understand it.  If you take a pencil and
> a sheet of paper and evaluate an example for yourself by hand, step by
> step, you'll understand how it works.  And no, this is not missing in
> the documentation.  The Emacs documentation is not an introduction into
> lambda calculus and writing LISP code.  Once you have really understood
> how evaluation of expressions in LISP works, you'll understand the above
> definition.

Why would you say it´s a definition?

When you consider "food is edible" as a definition, it doesn´t tell you
anything.

The function is defined in the source.  The documentation of it should
tell you what it does and how to make it do that.  Besides that the
documentation of advice-add doesn´t do that, advice-add is complicated,
convoluted and cryptic.

>> Considering all that, wouldn´t it be much better to create my own
>> mode?
>
> I don't think this is appropriate, because what you want can be done in
> few lines without making much assumptions about the original package.

Then what´s the purpose of modes, or the concept behind them?  As it is
now, I can enable a mode, like hi-lock-mode, no matter what kind of file
I´m editing.  The mode provides me with some particular functionality
which I can use.  It doesn´t matter to me how many lines of code it took
to create this mode.

Now I have some particular functionality I may want to use with many
different kinds of files.  So why not just enable a mode that gives me
this functionality?

What does it matter how the mode provides the functionality and how many
lines of code it´s made of?


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Florian Beck]

>>    (lambda (&rest r) (apply FUNCTION OLDFUN r))

This line tells you what the ADVICE does (in this case the around 
advice). But what does it mean?

1. Once you define an advice, instead of OLDFUN, emacs calls
(lambda (&rest r) (apply FUNCTION OLDFUN r)), which is a function that 
has any arguments and only calls (apply FUNCTION OLDFUN r)

2. &rest r means all (arbitrarily many) arguments are collected in the 
variable r.

3. apply then calls your FUNCTION with OLDFUN and the rest as arguments

For example, define a function:

(defun my-fun (a b)
   (list a b))

and a function we will add as an advice:

(defun my-advice (&rest args)
   args)

Add it:

(advice-add 'my-fun :around 'my-advice)

Now call (my-fun 'a 'b)

This returns ((lambda (a b) (list a b)) 1 2)

As you can see, your advice function gets called with three arguments of 
which the first is the original definition.

With that knowledge, we can redefine our advice function:

(defun my-advice (fun a b)
   (funcall fun a 'X))

using the first argument to call the original function.

And that is really all you have to know when advising a function

  - define a new function that takes the old function as an additional 
first argument

  - add the advice (advice-add 'my-fun :around 'my-advice)

Things are even easier with a :before advice, where you can do anything 
you want with the arguments:

(defun my-fun2 (a b)
   (list a b))

(defun my-advice2 (x y)
   (message "List contains %s and %s." x y))

(advice-add 'my-fun2 :before 'my-advice2)


-- 
Florian Beck

Re: replacing a function with another one

[Florian Beck]

> This returns ((lambda (a b) (list a b)) 1 2)

Of course: ((lambda (a b) (list a b)) a b)


-- 
Florian Beck

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> The function is defined in the source.  The documentation of it should
> tell you what it does and how to make it do that.  Besides that the
> documentation of advice-add doesn´t do that, advice-add is complicated,
> convoluted and cryptic.

It's only cryptic for you because you are not yet used to LISP and its
concepts.  This is no critic; the working of nadvice is just hardly
understandable for someone not knowing yet how `let' works.  This is
also no critic.  We have all started from the beginning, and I respect
that you start hacking with a quite ambitious project.  Once you are
having fun using higher order functions and such stuff, you'll see that
it makes some sense.

Michael.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>> The function is defined in the source.  The documentation of it should
>> tell you what it does and how to make it do that.  Besides that the
>> documentation of advice-add doesn´t do that, advice-add is complicated,
>> convoluted and cryptic.
>
> It's only cryptic for you because you are not yet used to LISP and its
> concepts.  This is no critic; the working of nadvice is just hardly
> understandable for someone not knowing yet how `let' works.  This is
> also no critic.  We have all started from the beginning, and I respect
> that you start hacking with a quite ambitious project.  Once you are
> having fun using higher order functions and such stuff, you'll see that
> it makes some sense.

Maybe in a few years ...  I just learn by doing --- I`m using emacs for
over 20 years and wish I had learned more in the past.  I never got to
it and didn`t bother much because it has always been working great.

Elisp is fun when you get used to it some, and it`s surprisingly
powerful.  I have created a mode from scratch yesterday and set up a
repo for things like that: https://github.com/lee-/emacs


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Stefan Monnier]

> Considering all that, wouldn´t it be much better to create my own mode?

Another good approach is to take hi-lock.el, modify it (as little as
possible) to provide the feature you want, and then to submit the patch
for inclusion in a future Emacs.


        Stefan

Re: replacing a function with another one

[lee]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Considering all that, wouldn´t it be much better to create my own mode?
>
> Another good approach is to take hi-lock.el, modify it (as little as
> possible) to provide the feature you want, and then to submit the patch
> for inclusion in a future Emacs.

I´d love to do that, and I think it could be done.  I´d mostly have to
add some things to it and probably wouldn´t need to modify what´s
already there.

How would I do that?  hi-lock.el is part of emacs, which is in a bzr
repo from which I update every now and then.  I could take the current
version of hi-lock.el and put it into a git repo to work on it.  In the
meantime, the original version may change.  Then what?


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Jambunathan K]

lee <lee@yun.yagibdah.de> writes:

>> Another good approach is to take hi-lock.el, modify it (as little as
>> possible) to provide the feature you want, and then to submit the patch
>> for inclusion in a future Emacs.
>
> I´d love to do that, and I think it could be done.  I´d mostly have to
> add some things to it and probably wouldn´t need to modify what´s
> already there.
>
> How would I do that?  hi-lock.el is part of emacs, which is in a bzr
> repo from which I update every now and then.  I could take the current
> version of hi-lock.el and put it into a git repo to work on it.  In the
> meantime, the original version may change.  Then what?

Where there is a will there is a way.

How to propose an emacs patch (Re: replacing a function with another one)

[lee]

Jambunathan K <kjambunathan@gmail.com> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>>> Another good approach is to take hi-lock.el, modify it (as little as
>>> possible) to provide the feature you want, and then to submit the patch
>>> for inclusion in a future Emacs.
>>
>> I´d love to do that, and I think it could be done.  I´d mostly have to
>> add some things to it and probably wouldn´t need to modify what´s
>> already there.
>>
>> How would I do that?  hi-lock.el is part of emacs, which is in a bzr
>> repo from which I update every now and then.  I could take the current
>> version of hi-lock.el and put it into a git repo to work on it.  In the
>> meantime, the original version may change.  Then what?
>
> Where there is a will there is a way.

Asking here how best to go about it is a start ...  It seems they need
some sort of paperwork from contributers for copyright reasons, and I
don`t know how to propose a patch or if such paperwork is needed for
that.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: How to propose an emacs patch (Re: replacing a function with another one)

[Jambunathan K]

lee <lee@yun.yagibdah.de> writes:

>> Where there is a will there is a way.
>
> Asking here how best to go about it is a start ...  It seems they need
> some sort of paperwork from contributers for copyright reasons, and I
> don`t know how to propose a patch or if such paperwork is needed for
> that.

You need to come to emacs-devel.  Second best option is to put it on
emacs-bugs.  Just say that you are proposing a patch and would like to
have it as part of Emacs.  Folks there will do the rest.

If you are new (or relatively new) to that list someone would respond
very quickly so that you feel quite welcome.  But as time progresses -
as you become a old timer - you may have to wait anywhere from few days
to few months for someone to turn attention to your work.

Emacs repo is quite big.  I suggest that you use git (and not bzr) for
cloning your repo.  You can find instructions here:

    http://www.emacswiki.org/emacs/GitForEmacsDevs


Remember you don't need to clone the repo for proposing a patch.  You
download JUST the relevant file from the Web Interface.  You make a copy
of it and make changes to it.  And then submit the diff between the
files.

emacs-devel:

    http://lists.gnu.org/archive/html/emacs-devel/
    http://thread.gmane.org/gmane.emacs.devel


emacs-bugs:

    http://lists.gnu.org/archive/html/bug-gnu-emacs/
    http://thread.gmane.org/gmane.emacs.bugs

emacs-sources.

    http://lists.gnu.org/archive/html/gnu-emacs-sources/
    http://thread.gmane.org/gmane.emacs.sources

You can more information about emacs and the mailing list in this page.

    http://savannah.gnu.org/projects/emacs
    https://savannah.gnu.org/bzr/?group=emacs

Re: How to propose an emacs patch (Re: replacing a function with another one)

[lee]

Jambunathan K <kjambunathan@gmail.com> writes:

> lee <lee@yun.yagibdah.de> writes:
>
>>> Where there is a will there is a way.
>>
>> Asking here how best to go about it is a start ...  It seems they need
>> some sort of paperwork from contributers for copyright reasons, and I
>> don`t know how to propose a patch or if such paperwork is needed for
>> that.
>
> You need to come to emacs-devel.

Hm, can`t hurt to subscribe :)

> Second best option is to put it on emacs-bugs.

I just did so --- I didn`t see your post earlier.

> Just say that you are proposing a patch and would like to
> have it as part of Emacs.  Folks there will do the rest.

We`ll see --- I might have some mistakes in it because I don`t know
any better.  It works fine for me, though and I hope they take it in.

You can try it out if you like: https://github.com/lee-/emacs/tree/master/hi-lock

> Remember you don't need to clone the repo for proposing a patch.  You
> download JUST the relevant file from the Web Interface.  You make a copy
> of it and make changes to it.  And then submit the diff between the
> files.

Yes, I sent a diff.  I`ve been updating and compiling from bzr every now
and then, so I switched to git and cloned the whole repo.  I get much
better along with git.

> emacs-devel:
> emacs-bugs:
> emacs-sources.

I`m subscribed to emacs-bugs since a while; I`ll get the others, too.


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[lee]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> lee <lee@yun.yagibdah.de> writes:
>>
>> (defadvice hi-lock-set-file-patterns (after
>> lsl-hi-lock-set-file-patterns-advice activate)
>>   (setq hi-lock-interactive-patterns (ad-get-arg 0)))
>
>> How would I do the same with add-advice (or whatever is appropriate)?
>
> (advice-add
>  'hi-lock-set-file-patterns :after
>  (lambda (patterns)
>    (setq hi-lock-interactive-patterns patterns)))
>
> You can also give the piece of advice a name like in defadvice:

I think I start to understand :)  You are creating an anonymous function
and attach it to hi-lock-set-file-patterns so it runs after it.

What are the equivalents of ad-deactivate and ad-activate with
advice-add?  I only found advice-add and advice-remove.  Is it possible
that an advice adds and removes itself instead of activating and
deactivating itself?


-- 
Knowledge is volatile and fluid.  Software is power.

Re: replacing a function with another one

[Michael Heerdegen]

lee <lee@yun.yagibdah.de> writes:

> What are the equivalents of ad-deactivate and ad-activate with
> advice-add?  I only found advice-add and advice-remove.

Yes, you can use these.  AFAIK, there is no direct replacement for
(de)activation.

> Is it possible that an advice adds and removes itself instead of
> activating and deactivating itself?

If you have named them so that you can refer to them, yes, that would
work.  Although it's much cleaner to use auxiliary variables as Stefan
described.

Michael.

Re: replacing a function with another one

[Jambunathan K]

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


Hello OP,

It seems you don't mind digging in to elisp code.

1. Put the following snippet (see below) in to your .emacs.
2. Restart emacs
3. C-x C-f somefile.txt
4. M-x add-file-local-variable RET hi-lock-patterns-file RET "somefile.patterns"
5. C-x C-v
6. Highlight some strings here and there
7. Make some modifications to the file
8. C-x C-s

Now,

1. C-x C-f somefile.txt.
2. You should see the highlights.

When you highlight some text, the buffer is considered as *not*
modified.  So after-save-hook will not be called if there are no actual
modifications in the current session. In that case, you can force a
write of the patterns file with

   M-x hi-lock-file-save-patterns

See the attached sample files for what I mean.

----------------------------------------------------------------

(add-hook 'find-file-hook 'hi-lock-find-file-hook)
(add-hook 'after-save-hook 'hi-lock-file-save-patterns)

(defun hi-lock-find-file-hook ()
  (when (and (boundp 'hi-lock-patterns-file)
	     (file-readable-p hi-lock-patterns-file))
    (let ((patterns (with-current-buffer (find-file-noselect hi-lock-patterns-file)
		      (goto-char (point-min))
		      (prog1 (ignore-errors (read (current-buffer)))
			(kill-buffer)))))
      (setq hi-lock-interactive-patterns patterns)
      (font-lock-add-keywords nil hi-lock-interactive-patterns t))))

(defun hi-lock-file-save-patterns ()
  (interactive)
  (when (boundp 'hi-lock-patterns-file)
    (let ((patterns (append hi-lock-file-patterns hi-lock-interactive-patterns)))
      (with-current-buffer (find-file-noselect hi-lock-patterns-file)
	(erase-buffer)
	(insert (pp-to-string patterns))
	(save-buffer 0)))))

----------------------------------------------------------------


[-- Attachment #2: somefile.patterns --]
[-- Type: application/octet-stream, Size: 141 bytes --]

(("\\_<goodbye\\_>"
  (0 'highlight-3 prepend))
 ("\\_<world\\_>"
  (0 'highlight-2 prepend))
 ("\\_<hello\\_>"
  (0 'highlight-1 prepend)))

[-- Attachment #3: somefile.txt --]
[-- Type: text/plain, Size: 105 bytes --]

hello world

goodbye world




;; Local Variables:
;; hi-lock-patterns-file: "somefile.patterns"
;; End:

Re: replacing a function with another one

[Jambunathan K]

Jambunathan K <kjambunathan@gmail.com> writes:

> 4. M-x add-file-local-variable RET hi-lock-patterns-file RET "somefile.patterns"
> 5. C-x C-v

Add a 4.5 step.

4.5 C-x C-s

Re: replacing a function with another one

[lee]

Jambunathan K <kjambunathan@gmail.com> writes:

> Hello OP,
>
> It seems you don't mind digging in to elisp code.

Well, I have to; It`s a great source to learn from :)

> ----------------------------------------------------------------
>
> (add-hook 'find-file-hook 'hi-lock-find-file-hook)
> (add-hook 'after-save-hook 'hi-lock-file-save-patterns)
>
> (defun hi-lock-find-file-hook ()
>   (when (and (boundp 'hi-lock-patterns-file)
> 	     (file-readable-p hi-lock-patterns-file))
>     (let ((patterns (with-current-buffer (find-file-noselect hi-lock-patterns-file)
> 		      (goto-char (point-min))
> 		      (prog1 (ignore-errors (read (current-buffer)))
> 			(kill-buffer)))))
>       (setq hi-lock-interactive-patterns patterns)
>       (font-lock-add-keywords nil hi-lock-interactive-patterns t))))
>
> (defun hi-lock-file-save-patterns ()
>   (interactive)
>   (when (boundp 'hi-lock-patterns-file)
>     (let ((patterns (append hi-lock-file-patterns hi-lock-interactive-patterns)))
>       (with-current-buffer (find-file-noselect hi-lock-patterns-file)
> 	(erase-buffer)
> 	(insert (pp-to-string patterns))
> 	(save-buffer 0)))))
>
> ----------------------------------------------------------------

I see what it does, thank you :)  I think I can use some of this when I
make an addition to hi-lock.el.


-- 
Knowledge is volatile and fluid.  Software is power.