Re: Adding new package org-menu to nongnu elpa

[Philip Kaludercic <philipk@posteo.net>]

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