Re: Adding new package org-menu to nongnu elpa

[Philip Kaludercic]

Jan Rehders <jan@sheijk.net> writes:

> Thanks for the feedback. Applied them. Some comments:
>
>> +    (_ (insert (format "unknown snippet type %s" snippet-id))))) ;or use 
>> `pcase-exhaustive'?
>
> Throwing an error here now to keep the info about what snippet type was not 
> found

1+

>> +;; You are require'ing yasnippet anyway, so there is no need to mess with 
>> autoloads
>
> yasnippet should only be required dynamically if it is selected so the plugin 
> can work without it being installed

Right, so it is wrong to autoload the function, as you do not want to
assume that they are loadable.

>> (defun org-menu-toggle-nbspace ()
>>   "Will remove non-breaking space before/after point or insert it if none 
>> found."
>> +  ;;           ^
>> +  ;;           Non-breaking or zero-width space?  \u200b is zero-width 
>> (used below)
>
> Yes that should have been zero-width space. Fixed

OK, btw. I remembered that you can simplify the eval-when-compile by
just writing

  ?\N{ZERO WIDTH SPACE}

.

>> +;; Can you explain why you are copying code org-colview.el?  It is not
>> +;; clear to me why you don't try to call it directly.
>
> This code used to be in lambdas and has since been moved into top-level 
> functions. Calling them now
>
> I’ve also changed the calls to deprecated functions you pointed
> out. This might break with older Emacs versions but those are
> supported as a best effort, only so let’s see if this fails for anyone
> or if compat.el handles this

Compat will probably not be of much use, as these were Org-specific
functions and Compat only provides core-Emacs functions.

> Thanks!
>

-- 
        Philip Kaludercic on siskin