search

bastibe / annotate.el

Annotate.el
Other
384 stars 20 forks source link

Info is a dependency #73

Closed articuluxe closed 4 years ago

articuluxe commented 4 years ago

The usage of Info-current-file means that I had to (require 'info) or encounter an unknown variable error. I would think this should just be added to the top of annotate.el.

cage2 commented 4 years ago

On Tue, Jun 30, 2020 at 08:30:01AM -0700, articuluxe wrote:

Hi articuluxe!

The usage of Info-current-file means that I had to (require 'info) or encounter an unknown variable error. I would think this should just be added to the top of annotate.el.

Wow, it is actually possible to install Emacs without info? Genuinely surprised about that!

I wonder what happens if an user already have loaded info before loading this package (so, loading info twice).

Now the problem is, is there a guarantee that code inside info was idempotent? For example i think loading two times annotate could generate some weird behaviour.

Is there a way to conditionally require a package?

I mean: if info is not loaded then (require 'info).

Should not be user responsibility to load info (in their init file) before loading annotate?

I this case i think i should patch the documentation to explicitly state that info should be loaded (even with the 'require' form you suggested) to use this package.

I think i need some help here. :)

Bye! C.

EDIT: added word

bastibe commented 4 years ago

I think multiple require do not cause problems. It's simply a no-op if it has been loaded elsewhere.

But it's true, if we use info functionality, we need to require it first.

cage2 commented 4 years ago

On Wed, Jul 01, 2020 at 12:14:20AM -0700, Bastian Bechtold wrote:

Hello Bastian!

I think multiple require do not cause problems. It's simply a no-op if it has been loaded elsewhere.

Good, my only doubt was if it was harmful to load it multiple times.

But it's true, if we use info functionality, we need to require it first.

Ok i am going to patch the code with the required "require". ;)

Bye! C.

cage2 commented 4 years ago

Hi @articuluxe!

I think the bug you reported is fixed after merging #75 , thank you for reporting! :)

Bye! C.

articuluxe commented 4 years ago

Thank you!

On Jul 3, 2020, at 9:49 AM, cage2 notifications@github.com wrote:

Hi @articuluxe https://github.com/articuluxe!

I think the bug you reported is fixed after merging #75 https://github.com/bastibe/annotate.el/pull/75 , thank you for reporting! :)

Bye! C.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bastibe/annotate.el/issues/73#issuecomment-653582032, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYO2SP3GRKJX45ZLZJJAE3RZXVWZANCNFSM4OMLLOJQ.

cage2 commented 4 years ago

On Fri, Jul 03, 2020 at 09:26:33AM -0700, articuluxe wrote:

Thank you!

You're welcome! :)

C.