unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Bug solved
@ 2006-11-29 14:12 A Soare
  2006-11-29 18:46 ` Masatake YAMATO
  2006-11-30 11:47 ` Masatake YAMATO
  0 siblings, 2 replies; 6+ messages in thread
From: A Soare @ 2006-11-29 14:12 UTC (permalink / raw)


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

BUG:

> 1) open an arbitrary file
>    C-x C-f /root/.emacs for example
> 2) suppose highlight current line minor mode is on. Sinon:
>    (global-hl-line-mode 1)
> 3) (hexl-mode)
> 4) (hexl-mode-exit)
>
> BUG: (hexl-mode-exit) forgot to change again the highlight, and we see the normal text with the highl. from hex mode.
>
> version: GNU Emacs 22.0.91.1 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars) of 2006-11-24 on alin.ro
> system: FC6: Linux alin.ro 2.6.18-1.2798.fc6 #1 SMP Mon Oct 16 14:37:32 EDT 2006 i686 athlon i386 GNU/Linux
> 

I attached here
- the output from cvs -d:pserver:anonymous@cvs.gnu.org:/sources/emacs diff -c emacs/lisp/hexl.el
- a lisp/changelog entry
- a little text file with comments

Please inform me if you accepted the solution.

Best regards,

Alin Soare

[-- Attachment #2: diff-bug-hex-hl-line-emacs --]
[-- Type: application/octet-stream, Size: 1605 bytes --]

Index: emacs/lisp/hexl.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/hexl.el,v
retrieving revision 1.107
diff -c -r1.107 hexl.el
*** emacs/lisp/hexl.el	23 Nov 2006 18:34:44 -0000	1.107
--- emacs/lisp/hexl.el	29 Nov 2006 13:24:28 -0000
***************
*** 104,109 ****
--- 104,111 ----
  (defvar ruler-mode-ruler-function)
  (defvar hl-line-mode)
  
+ (defvar hexl-mode-old-hl-line-range-function)
+ (defvar hexl-mode-old-hl-line-face)
  (defvar hexl-mode-old-hl-line-mode)
  (defvar hexl-mode-old-local-map)
  (defvar hexl-mode-old-mode-name)
***************
*** 259,264 ****
--- 261,271 ----
      (setq hexl-mode-old-hl-line-mode
  	  (and (boundp 'hl-line-mode) hl-line-mode))
  
+     (set (make-local-variable 'hexl-mode-old-hl-line-range-function)
+          hl-line-range-function)
+     (set (make-local-variable 'hexl-mode-old-hl-line-face)
+          hl-line-face)
+ 
      (make-local-variable 'hexl-mode-old-syntax-table)
      (setq hexl-mode-old-syntax-table (syntax-table))
      (set-syntax-table (standard-syntax-table))
***************
*** 388,393 ****
--- 395,404 ----
        (ruler-mode 0))
    (if (and (boundp 'hl-line-mode) hl-line-mode (not hexl-mode-old-hl-line-mode))
        (hl-line-mode 0))
+ 
+   (set 'hl-line-range-function hexl-mode-old-hl-line-range-function)
+   (set 'hl-line-face hexl-mode-old-hl-line-face)
+ 
    (setq require-final-newline hexl-mode-old-require-final-newline)
    (setq mode-name hexl-mode-old-mode-name)
    (setq isearch-search-fun-function hexl-mode-old-isearch-search-fun-function)

[-- Attachment #3: commentary-bug-hex-hl-line-emacs --]
[-- Type: application/octet-stream, Size: 970 bytes --]

The hook function from the hexl-mode changed the value of the
hl-line-range-function variable, by assingning it the symbol
'hexl-highlight-line-range, called from hl-line.el:

(defun hl-line-move (overlay) ()
  "Move the Hl-Line overlay.
If `hl-line-range-function' is non-nil, move the OVERLAY to the position
where the function returns.  If `hl-line-range-function' is nil, fill
the line including the point by OVERLAY."
...
	(setq tmp (funcall hl-line-range-function)
...)

Before exit from the hexl-modecalling (hexl-mode-exit), the
old value of the hl-line-range-function was not restored.

That is what I did: to restore its value.

I declared a buffer-local variable in (hexl-mode) which keeps the old
value of hl-line-range-function, and in hexl-mode-exit I restored its
initial value.

Another solution would have been that (hexlify-buffer) put its
output into another buffer (without killing initial buffer). But for
this sol. there are lots of changes to do.

[-- Attachment #4: Changelog.alin-soare --]
[-- Type: application/octet-stream, Size: 263 bytes --]

2006-11-29  Alin C. Soare  <alinsoar@voila.fr>

	* lisp/hexl.el (hexl-mode-old-hl-line-range-function): New variable
	(hexl-mode-old-hl-line-face): New variable
	(hexl-mode, hexl-mode-exit): Fix the highlighting of the current
	line when exit from the hexl-mode.

[-- Attachment #5: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Bug solved
  2006-11-29 14:12 Bug solved A Soare
@ 2006-11-29 18:46 ` Masatake YAMATO
  2006-11-30 11:47 ` Masatake YAMATO
  1 sibling, 0 replies; 6+ messages in thread
From: Masatake YAMATO @ 2006-11-29 18:46 UTC (permalink / raw)
  Cc: emacs-devel

> BUG:
> 
> > 1) open an arbitrary file
> >    C-x C-f /root/.emacs for example
> > 2) suppose highlight current line minor mode is on. Sinon:
> >    (global-hl-line-mode 1)
> > 3) (hexl-mode)
> > 4) (hexl-mode-exit)
> >
> > BUG: (hexl-mode-exit) forgot to change again the highlight, and we see the normal text with the highl. from hex mode.
> >
> > version: GNU Emacs 22.0.91.1 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars) of 2006-11-24 on alin.ro
> > system: FC6: Linux alin.ro 2.6.18-1.2798.fc6 #1 SMP Mon Oct 16 14:37:32 EDT 2006 i686 athlon i386 GNU/Linux
> > 
> 
> I attached here
> - the output from cvs -d:pserver:anonymous@cvs.gnu.org:/sources/emacs diff -c emacs/lisp/hexl.el
> - a lisp/changelog entry
> - a little text file with comments
> 
> Please inform me if you accepted the solution.
> 
> Best regards,
> 
> Alin Soare

Maybe I introduced this bug. So I'd like to review your patch.
Give me two or three days.

Thank you.
Masatake YAMATO

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Bug solved
  2006-11-29 14:12 Bug solved A Soare
  2006-11-29 18:46 ` Masatake YAMATO
@ 2006-11-30 11:47 ` Masatake YAMATO
  2006-11-30 13:33   ` Kim F. Storm
  1 sibling, 1 reply; 6+ messages in thread
From: Masatake YAMATO @ 2006-11-30 11:47 UTC (permalink / raw)
  Cc: emacs-devel

> BUG:
> 
> > 1) open an arbitrary file
> >    C-x C-f /root/.emacs for example
> > 2) suppose highlight current line minor mode is on. Sinon:
> >    (global-hl-line-mode 1)
> > 3) (hexl-mode)
> > 4) (hexl-mode-exit)
> >
> > BUG: (hexl-mode-exit) forgot to change again the highlight, and we see the normal text with the highl. from hex mode.
> >
> > version: GNU Emacs 22.0.91.1 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars) of 2006-11-24 on alin.ro
> > system: FC6: Linux alin.ro 2.6.18-1.2798.fc6 #1 SMP Mon Oct 16 14:37:32 EDT 2006 i686 athlon i386 GNU/Linux
> > 
> 
> I attached here
> - the output from cvs -d:pserver:anonymous@cvs.gnu.org:/sources/emacs diff -c emacs/lisp/hexl.el
> - a lisp/changelog entry
> - a little text file with comments
> 
> Please inform me if you accepted the solution.

Thank you for reporting bug and fixing it.
I reviewed your patch and modified it.
Generally your patch is good. However, the case that hl-line-range-function and/or
hl-line-face is/are not bound to value(s) is not handled.
So I modified your patch to handle the case.
I also did the same on ruler related code.
Could you test my patch?

I think it is almost ready to install the patch to the cvs repository.
However, I think about copyrigh assignment.
Other developers, sign for copyrigh assignment from Alin Soare is needed
in this case? Without white lines, Alin's original patch contains 6 lines.

Masatake YAMATO

Index: hexl.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/hexl.el,v
retrieving revision 1.107
diff -u -r1.107 hexl.el
--- hexl.el	23 Nov 2006 18:34:44 -0000	1.107
+++ hexl.el	30 Nov 2006 11:34:14 -0000
@@ -100,15 +100,22 @@
 
 (defvar hexl-mode-map nil)
 
+;; Variable declarations for suppressing warnings from the byte-compiler.
 (defvar ruler-mode)
 (defvar ruler-mode-ruler-function)
 (defvar hl-line-mode)
+(defvar hl-line-range-function)
+(defvar hl-line-face)
 
+;; Variables where the original values are stored to.
 (defvar hexl-mode-old-hl-line-mode)
+(defvar hexl-mode-old-hl-line-range-function)
+(defvar hexl-mode-old-hl-line-face)
 (defvar hexl-mode-old-local-map)
 (defvar hexl-mode-old-mode-name)
 (defvar hexl-mode-old-major-mode)
 (defvar hexl-mode-old-ruler-mode)
+(defvar hexl-mode-old-ruler-function)
 (defvar hexl-mode-old-isearch-search-fun-function)
 (defvar hexl-mode-old-require-final-newline)
 (defvar hexl-mode-old-syntax-table)
@@ -386,8 +393,16 @@
 
   (if (and (boundp 'ruler-mode) ruler-mode (not hexl-mode-old-ruler-mode))
       (ruler-mode 0))
+  (when (boundp 'hexl-mode-old-ruler-function)
+    (setq ruler-mode-ruler-function hexl-mode-old-ruler-function))
+
   (if (and (boundp 'hl-line-mode) hl-line-mode (not hexl-mode-old-hl-line-mode))
       (hl-line-mode 0))
+  (when (boundp 'hexl-mode-old-hl-line-range-function)
+    (setq hl-line-range-function hexl-mode-old-hl-line-range-function))
+  (when (boundp hexl-mode-old-hl-line-face)
+    (setq hl-line-face hexl-mode-old-hl-line-face))
+ 
   (setq require-final-newline hexl-mode-old-require-final-newline)
   (setq mode-name hexl-mode-old-mode-name)
   (setq isearch-search-fun-function hexl-mode-old-isearch-search-fun-function)
@@ -929,19 +944,26 @@
 (defun hexl-activate-ruler ()
   "Activate `ruler-mode'."
   (require 'ruler-mode)
+  (unless (boundp 'hexl-mode-old-ruler-function)
+    (set (make-local-variable 'hexl-mode-old-ruler-function)
+	 ruler-mode-ruler-function))
   (set (make-local-variable 'ruler-mode-ruler-function)
        'hexl-mode-ruler)
   (ruler-mode 1))
 
 (defun hexl-follow-line ()
   "Activate `hl-line-mode'."
-  (require 'frame)
   (require 'hl-line)
-  (with-no-warnings
-    (set (make-local-variable 'hl-line-range-function)
-	 'hexl-highlight-line-range)
-    (set (make-local-variable 'hl-line-face)
-	 'highlight))
+  (unless (boundp 'hexl-mode-old-hl-line-range-function)
+    (set (make-local-variable 'hexl-mode-old-hl-line-range-function)
+	 hl-line-range-function))
+  (unless (boundp 'hexl-mode-old-hl-line-face)
+    (set (make-local-variable 'hexl-mode-old-hl-line-face)
+	 hl-line-face))
+  (set (make-local-variable 'hl-line-range-function)
+       'hexl-highlight-line-range)
+  (set (make-local-variable 'hl-line-face)
+       'highlight)
   (hl-line-mode 1))
 
 (defun hexl-highlight-line-range ()

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Bug solved
  2006-11-30 11:47 ` Masatake YAMATO
@ 2006-11-30 13:33   ` Kim F. Storm
  2006-11-30 16:12     ` Masatake YAMATO
  0 siblings, 1 reply; 6+ messages in thread
From: Kim F. Storm @ 2006-11-30 13:33 UTC (permalink / raw)
  Cc: alinsoar, emacs-devel

Masatake YAMATO <jet@gyve.org> writes:

> I think it is almost ready to install the patch to the cvs repository.
> However, I think about copyrigh assignment.
> Other developers, sign for copyrigh assignment from Alin Soare is needed
> in this case? Without white lines, Alin's original patch contains 6 lines.

It is ok to checkin Alin's original patch with a separate change log entry,
and mark is as a (tiny change).

Then you can install your patch after that with your own change log entry.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Bug solved
  2006-11-30 13:33   ` Kim F. Storm
@ 2006-11-30 16:12     ` Masatake YAMATO
  0 siblings, 0 replies; 6+ messages in thread
From: Masatake YAMATO @ 2006-11-30 16:12 UTC (permalink / raw)
  Cc: alinsoar, emacs-devel

> > I think it is almost ready to install the patch to the cvs repository.
> > However, I think about copyrigh assignment.
> > Other developers, sign for copyrigh assignment from Alin Soare is needed
> > in this case? Without white lines, Alin's original patch contains 6 lines.
> 
> It is ok to checkin Alin's original patch with a separate change log entry,
> and mark is as a (tiny change).
> 
> Then you can install your patch after that with your own change log entry.

I've just installed Alin's original patch and my patch.

Thank you.

Masatake YAMATO

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Bug solved
@ 2006-12-01 14:51 A Soare
  0 siblings, 0 replies; 6+ messages in thread
From: A Soare @ 2006-12-01 14:51 UTC (permalink / raw)
  Cc: emacs-devel


Besides, I ask myself whether it is a good idea to stop the debugger when one enters the hexl mode (in (hexlify-buffer)). Anyway, the debugger is unuseful into hexified buffer.

Veuillez agreer mes meilleures salutations.

Alin Soare.

> Message du 30/11/06 à 17h12
> De : "Masatake YAMATO" <jet@gyve.org>
> A : storm@cua.dk
> Copie à : alinsoar@voila.fr, emacs-devel@gnu.org
> Objet : Re: Bug solved
> 
> > > I think it is almost ready to install the patch to the cvs repository.
> > > However, I think about copyrigh assignment.
> > > Other developers, sign for copyrigh assignment from Alin Soare is needed
> > > in this case? Without white lines, Alin's original patch contains 6 lines.
> > 
> > It is ok to checkin Alin's original patch with a separate change log entry,
> > and mark is as a (tiny change).
> > 
> > Then you can install your patch after that with your own change log entry.
> 
> I've just installed Alin's original patch and my patch.
> 
> Thank you.
> 
> Masatake YAMATO
> 
> 


Liberté, Égalité, Jules Rimet!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-12-01 14:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-29 14:12 Bug solved A Soare
2006-11-29 18:46 ` Masatake YAMATO
2006-11-30 11:47 ` Masatake YAMATO
2006-11-30 13:33   ` Kim F. Storm
2006-11-30 16:12     ` Masatake YAMATO
  -- strict thread matches above, loose matches on Subject: below --
2006-12-01 14:51 A Soare

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).