[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Optimizations for flymake
From: |
Kim F. Storm |
Subject: |
Re: Optimizations for flymake |
Date: |
Tue, 02 Nov 2004 10:10:45 +0100 |
User-agent: |
Gnus/5.11 (Gnus v5.11) Emacs/21.3.50 (gnu/linux) |
Stefan <address@hidden> writes:
>> ! (defsubst flymake-makehash(&optional test)
>> ! (if (featurep 'xemacs)
>> ! (if test (make-hash-table :test test) (make-hash-table))
>> ! (makehash test)))
>
> Why not (if (fboundp 'make-hash-table) ...) ?
Right, that's better.
>
>> (defun flymake-float-time()
>> ! (if (featurep 'xemacs)
>> ! (let ((tm (current-time)))
>> ! (multiple-value-bind (s0 s1 s2) (current-time)
>> ! (+ (* (float (ash 1 16)) s0) (float s1) (*
>> 0.0000001 s2))))
>> ! (float-time)))
>
> Why not (if (fboundp 'float-time) ...) ?
>
Because Emacs doesn't have multiple-value-bind.
>> ! (defsubst flymake-replace-regexp-in-string(regexp rep str)
>> ! (if (featurep 'xemacs)
>> ! (replace-in-string str regexp rep)
>> ! (replace-regexp-in-string regexp rep str)))
>
> Why not (if (fboundp 'replace-regexp-in-string) ...) ?
> I'd actually make it even more efficient than `defsubst':
Like this?
(defalias 'flymake-replace-regexp-in-string
(if (fboundp 'replace-regexp-in-string))
'replace-regexp-in-string
'replace-in-string))
>
>> ! (defsubst flymake-split-string(str pattern)
>> ! (if (featurep 'xemacs)
>> ! (flymake-split-string-remove-empty-edges str pattern)
>> ! (flymake-split-string-remove-empty-edges str pattern)))
>
> Hmmm... isn't this just
> (defalias 'flymake-split-string 'flymake-split-string-remove-empty-edges) ?
Yes -- looks odd.
>
>> ! (defsubst flymake-get-temp-dir()
>> ! (if (featurep 'xemacs)
>> ! (temp-directory)
>> ! temporary-file-directory))
>
> Why not (if (fboundp 'temp-directory) ...) ?
Would work as well -- but it would hide why we make the check...
>
>> (defun flymake-popup-menu(pos menu-data)
>> ! (if (featurep 'xemacs)
>> ! (let* ((x-pos (nth 0 (nth 0 pos)))
>> ! (y-pos (nth 1 (nth 0 pos)))
>> ! (fake-event-props '(button 1 x 1 y 1)))
>> ! (setq fake-event-props (plist-put fake-event-props 'x x-pos))
>> ! (setq fake-event-props (plist-put fake-event-props 'y y-pos))
>> ! (popup-menu (flymake-make-xemacs-menu menu-data) (make-event
>> 'button-press fake-event-props))
>> ! )
>> ! (x-popup-menu pos (flymake-make-emacs-menu menu-data))))
>
> Since Emacs-21 has popup-menu, we should be able to throw away
> flymake-make-emacs-menu and rename flymake-make-xemacs-menu to
> flymake-make-menu:
>
> (defun flymake-popup-menu(pos menu-data)
> (popup-menu (flymake-make-menu menu-data)
> (if (not (featurep 'xemacs))
> pos
> (let* ((x-pos (nth 0 (nth 0 pos)))
> (y-pos (nth 1 (nth 0 pos)))
> (fake-event-props '(button 1 x 1 y 1)))
> (setq fake-event-props (plist-put fake-event-props 'x x-pos))
> (setq fake-event-props (plist-put fake-event-props 'y y-pos))
> (make-event 'button-press fake-event-props)))))
I take your word for it :-)
>
>> (defun flymake-current-row()
>> "return current row in current frame"
>> ! (if (featurep 'xemacs)
>> ! (count-lines (window-start) (point))
>> ! (+ (car (cdr (window-edges))) (count-lines (window-start) (point)))))
>
> I suspect this should be:
>
> (defun flymake-current-row()
> "return current row in current frame"
> (+ (count-lines (window-start) (point))
> (if (fboundp 'window-edges)
> (car (cdr (window-edges)))
> ;; On XEmacs we should probably use something else, but what??
> 0)))
Ok.
>
>> ! (defsubst flymake-selected-frame()
>> ! (if (featurep 'xemacs)
>> ! (selected-window)
>> ! (selected-frame)))
>
> XEmacs has `selected-frame' as well, so the above code looks odd.
> I must be missing something.
Me too.
It also occurred to me that rather than all the autoloads for the
various overlay things, it should simply do:
(require 'overlay)
Emacs has (featurep 'overlay) => t, so there's no need to
condition it with (featurep 'xemacs) either.
--
Kim F. Storm <address@hidden> http://www.cua.dk