[ELPA] New package: scanner

[ELPA] New package: scanner

[Raffael Stocker]

Hi,

I have made a simple document scanning package that I would like to
contribute to ELPA.  The source can be found here:
https://gitlab.com/rstocker/scanner.git

The package uses scanimage (GPLv2) from the SANE backends distribution
for scanning and tesseract (Apache License V2.0) for PDF generation and
OCR.

I have signed the copyright release forms a while ago.

Regards,
    Raffael

Re: [ELPA] New package: scanner

[Stefan Monnier]

> I have made a simple document scanning package that I would like to
> contribute to ELPA.  The source can be found here:
> https://gitlab.com/rstocker/scanner.git

Thanks, added.

Beside some technical details I had to tweak as part of the addition,
I noticed another problem: you add a submenu to the global menus (which
I guess is fine), but some of the commands in that menu are not
autoloaded so selecting them signals an error.  You should probably
"disable/grey" them until the package is loaded, or add autoloads, or
...

I'm also wondering why you need all those `:key-sequence nil` in the menu.
Maybe it reflects a bug in our code?


        Stefan

Re: [ELPA] New package: scanner

[Raffael Stocker]

Stefan Monnier writes:

> Beside some technical details I had to tweak as part of the addition,
> I noticed another problem: you add a submenu to the global menus (which
> I guess is fine),

TBH, I wasn't quite sure where to add my menu.  But I'm open to better
suggestions.

> but some of the commands in that menu are not
> autoloaded so selecting them signals an error.  You should probably
> "disable/grey" them until the package is loaded, or add autoloads, or
> ...

Thanks for spotting this, I will add autoloads.


> I'm also wondering why you need all those `:key-sequence nil` in the menu.
> Maybe it reflects a bug in our code?

The Elisp doc says in (info "(elisp) Extended Menu Items"): 

‘:key-sequence nil’
     This property indicates that there is normally no key binding which
     is equivalent to this menu item.  Using this property saves time in
     preparing the menu for display, because Emacs does not need to
     search the keymaps for a keyboard equivalent for this menu item.

I just thought this is how it's done.

  Raffael

Re: [ELPA] New package: scanner

[Raffael Stocker]

> Thanks for spotting this, I will add autoloads.

Ok, I have added them and also changed the version number to 0.1.  Could
you please merge this?

    Raffael

Re: [ELPA] New package: scanner

[Stefan Monnier]

> Ok, I have added them and also changed the version number to 0.1.  Could
> you please merge this?

Thanks, updated,


        Stefan

Re: [ELPA] New package: scanner

[Stefan Monnier]

>> I'm also wondering why you need all those `:key-sequence nil` in the menu.
>> Maybe it reflects a bug in our code?
> The Elisp doc says in (info "(elisp) Extended Menu Items"): 
> ‘:key-sequence nil’
>      This property indicates that there is normally no key binding which
>      is equivalent to this menu item.  Using this property saves time in
>      preparing the menu for display, because Emacs does not need to
>      search the keymaps for a keyboard equivalent for this menu item.

AFAIK we never bother with this optimization.  Finding the key-sequence
is supposed to be very cheap (basically a hash-table lookup) because we
pre-compute a "reverse table" from commands to key-bindings.
[ This pre-computation is not that cheap but we the above keyword
  argument doesn't prevent its computation.  ]

> I just thought this is how it's done.

I guess we should adjust the doc, thanks.


        Stefan

Re: [ELPA] New package: scanner

[Raffael Stocker]

Stefan Monnier writes:

> AFAIK we never bother with this optimization.  Finding the key-sequence
> is supposed to be very cheap (basically a hash-table lookup) because we
> pre-compute a "reverse table" from commands to key-bindings.
> [ This pre-computation is not that cheap but we the above keyword
>   argument doesn't prevent its computation.  ]

Good to know, thanks for the explanation.

     Raffael