search

xahlee / xah-fly-keys

the most efficient keybinding for emacs
http://xahlee.info/emacs/misc/xah-fly-keys.html
483 stars 83 forks source link

Adding Xah Fly Keys to NonGNU ELPA #139

Closed phikal closed 2 years ago

phikal commented 2 years ago

Hi,

I wanted to ask if you are interested in having xah-fly-keys added to NonGNU ELPA?

xahlee commented 2 years ago

sure. I hope I don't have to do anything much. It's GPL3. Are you from GNU emacs dev?

phikal commented 2 years ago

I hope I don't have to do anything much.

Everything seems to build fine, but there are a few points I would like to mention:

None of these things are deal breakers, and I know you have your preferences, but if interested, I could prepare a few patches to merge before adding the package to the archive.

Are you from GNU emacs dev?

I'm just a contributor.

xahlee commented 2 years ago

i started to fix the long line docstring... but felt that's really going backward. Before, perfectly semantic unit sentences are now mutilated. And in rage thinking of just adding (setq byte-compile-docstring-max-column 999) to the top. lol.

i now added autoload.

Your commentary section seems to mix installation instructions with usage. When previewing a package using C-h P or on the ELPA website, the manually installation instructions are less relevant.

how to fix this? Could you give a tip? Is is specal comment string i need to use? Or, remove the manual install instruction, or put it at bottom?

If I understand correctly, you use these /;; s+-+/ comments as section separators. Would you consider adding a outline-regexp local variable to ease navigation?

that seems add more complexity. I can just replace them all with emacs tradition of ^L

i haven't commited to github yet. Please let me know your thoughts and i'll continue to fix them. I'll fix the long line doc string issue.

phikal commented 2 years ago

And in rage thinking of just adding (setq byte-compile-docstring-max-column 999) to the top. lol.

Actually byte-compile-docstring-max-column is a file local variable so this would work ^^

how to fix this? Could you give a tip? Is is specal comment string i need to use? Or, remove the manual install instruction, or put it at bottom?

ELPA extracts the content between ;;; Commentary: and the next section. So if we were to pull out the installation instructions and add that under a ;;; Installation: section, then that should fix the problem.

that seems add more complexity. I can just replace them all with emacs tradition of ^L

That would be just as fine.

xahlee commented 2 years ago

thanks a lot. I did a push. Let me know it's fine. The long line docstring i still need to fix. putting (setq byte-compile-docstring-max-column 999) in the file doesn't seems to help, as compile will not eval it... adding it as file local var declaration seems making it more complex. I'd just make it 80 char per line then.

please let me know if at least the install/usage instruction part is fixed.

phikal commented 2 years ago

Xah Lee @.***> writes:

thanks a lot. I did a push. Let me know it's fine. The long line docstring i still need to fix. putting (setq byte-compile-docstring-max-column 999) in the file doesn't seems to help, as compile will not eval it... adding it as file local var declaration seems making it more complex.

Do you think so? All you really need is

    ;; Local Variables:
    ;; byte-compile-docstring-max-column: 999
    ;; End:

which doesn't seem that bad. Setting byte-compile-docstring-max-column directly is certainly wrong, because 1. the variable assignment is evaluated when the package is loaded, not compiled, 2. loading the package has an unintended side effect.

I'd just make it 80 char per line then.

I see you have attempted to do this for a few variables and functions (e.g. `xah-fly--current-layout-kmap') by breaking the line at about the 80th column. This defeats the purpose of the checkdoc complaint, as the motivation behind it is to ensure that the first line is a sentence that makes sense on it's own when presented via apropos. It doesn't seem worth the effort to do this if the intention is missed, so either the documentation strings should be reworded (which I understand can be bothersome) or you can just let it be.

please let me know if at least the install/usage instruction part is fixed.

That looks fine. This is a render of how the HTML would look like on the ELPA website: https://amodernist.com/files/xah-fly-keys.html.

-- Philip Kaludercic

xahlee commented 2 years ago

thanks. pushed. All fixed i think.

PS

the ELPA website: https://amodernist.com/files/xah-fly-keys.html.

at the end of that doc, it says copyright by FSF. It means by having it in nonGnu Elpa, i assign copyright to FSF? ok. I guess it doesn't matter to me. please go ahead. Thanks.

phikal commented 2 years ago

Xah Lee @.***> writes:

thanks. pushed. All fixed i think.

Ok, finally I should mention the rules for inclusion, found here:

https://git.savannah.gnu.org/cgit/emacs/nongnu.git/tree/README.org#n133

Please take a look if you find anything objectionable, but in most cases there shouldn't be any issues as long as you don't rely on some hard-coded non-free dependency.

PS

the ELPA website: https://amodernist.com/files/xah-fly-keys.html. at the end of that doc, it says copyright by FSF. It means by having it in nonGnu Elpa, i assign copyright to FSF? ok. I guess it doesn't matter to me. please go ahead. Thanks.

No, AFAIK that is just the copyright for the page itself, not the contents of the package.

-- Philip Kaludercic

xahlee commented 2 years ago

great. and good to know the copyright doesn't auto become FSF. Anything else? If all good, feel free to close. Thank you very much.

phikal commented 2 years ago

No, that should be it. Thank you for your cooperation and all you do!

phikal commented 2 years ago

The package has been added and can now be downloaded fron NonGNU ELPA: http://elpa.nongnu.org/nongnu/xah-fly-keys.html