Re: Screenshots, frame shots straight from Emacs

[Tomas Hlavaty <tom@logand.com>]

Hi Leo,

On Tue 20 Jul 2021 at 12:42, Leo Butler <leo.butler@umanitoba.ca> wrote:
> Tomas Hlavaty <tom@logand.com> writes:
>
>> this one works with plain console emacs:
>> https://logand.com/sw/emacs-framebuffer/file/emacs-framebuffer.el.html#l755
>
> Nice. I have not tried it yet, but I do notice that

thanks for your feedback!

> https://logand.com/sw/emacs-framebuffer/file/emacs-framebuffer.el.html#l28
>
> requires cl.
>
> Also, when byte-compiling the file, I ran into 3 warnings which are easy
> to correct (see attached). I still haven't got it working, but I think
> that is due to user error.

What Emacs version are you using?

In Emacs 27.2, I get 2 warnings:

emacs-framebuffer.el:28:1:Warning: cl package required at runtime

and

In framebuffer-hh:mm:ss:
emacs-framebuffer.el:666:38:Warning: reference to free variable ‘position’

The first one does not break anything.

The second one is a bug relevant only for viewing videos on the console
from a chosen position.

None of them break screenshot functionality.

There is /dev/fb0 hardcoded so this might cause problems for some but I
actually never experienced any other device.  It would be interesting to
know, how to properly detect which device is actually used if there are
more than one.

> -(require 'cl)
> +(require 'cl-lib)

What is the difference?  Why is that needed?  Is it it backward
compatible?  There have been changes to working with cl package for some
time, has the dust settled yet?

> +(make-variable-buffer-local
> + (defvar framebuffer-image-mode-current-page))
> +
>  (defun framebuffer-draw (x y w h sx sy sw sh file)
>    (setq file (framebuffer-cache-remote file))
>    (let ((format (framebuffer-file-format file)))
> @@ -465,8 +468,6 @@
>   (defvar framebuffer-image-mode-image-size))
>  (make-variable-buffer-local
>   (defvar framebuffer-image-mode-npages))
> -(make-variable-buffer-local
> - (defvar framebuffer-image-mode-current-page))

I seems to be necessary to have these defined before use so I will
probably move those to the beginning of the file.

I did not get a warning for this.  Did you get a warning?

> -       (format "%d:%02d:%02d" (floor position 3600) (floor (- m s) 60) s)))))
> +       (format "%d:%02d:%02d" (floor sec 3600) (floor (- m s) 60) s)))))

Thanks, I'll fix this typo.

Tomas