Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate keybinding documentation #162

Merged
merged 6 commits into from
Aug 5, 2019
Merged

Generate keybinding documentation #162

merged 6 commits into from
Aug 5, 2019

Conversation

raboof
Copy link
Owner

@raboof raboof commented Jul 31, 2019

So we can show docs like https://notionwm.net/notionkeys.html that actually correspond to the current configuration

@raboof raboof marked this pull request as ready for review July 31, 2019 17:51
if ref:sub(1,5) == "Mod4+" then
ref = ref:sub(6, #ref)
end
io.write("<tr class=\"shortcut\" ref=\"#" .. ref .. "\"><td>" .. binding["kcb"] .. "</td><td>" .. binding["doc"] .. "</td></tr>\n")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you could save some backslashes by using single quotes?

io.write("<td width=\"400px\" valign=\"top\">\n")
io.write("<h2>META key</h2>\n")
io.write("All keybindings are activated by also pressing the META key, which by default is the 'windows' key.<br>\n")
io.write("The 'grey' bindings are activated by pressing Shift in addition to META.<br><br>\n")
Copy link
Collaborator

@knixeur knixeur Aug 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should read "The 'grey' bindings are activated by pressing ALTMETA (by default pressing Shift while holding META.<br><br>\n"

@raboof
Copy link
Owner Author

raboof commented Aug 3, 2019

Great review comments! Should all be done now.

@raboof raboof requested review from wilhelmy and knixeur August 3, 2019 15:16
@wilhelmy
Copy link
Collaborator

wilhelmy commented Aug 4, 2019

I think there's some cases of missing 'local' keywords left.. I've seen keys = 'foo' at least.

@wilhelmy
Copy link
Collaborator

wilhelmy commented Aug 4, 2019

I guess otherwise it's good to go. Cool new feature!

@wilhelmy
Copy link
Collaborator

wilhelmy commented Aug 4, 2019

I think there's some cases of missing 'local' keywords left.. I've seen keys = 'foo' at least.

Disregard this, of course you only need to declare the first assignment local.

@@ -100,9 +100,8 @@ end

function ioncore.show_live_docs(frame)
local keysfile = io.open("/tmp/notionkeys.html", "w")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could break if more than one user on the system uses notion, or if another user created the file already. Not sure it's really a concern.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this to use os.tmpname() (as io.tmpfile() does not necessarily have a name). I guess ideally we should clean up those files when exiting, but at least on linux /tmp will be cleared on reboot anyway, so this might be fine for now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know, I only knew about io.tmpfile(). The only problem is that the filename doesn't end with .html now... So maybe it might be required to call os.rename(filename, filename..".html") before opening, but if you say it works, I'll believe you :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works with firefox, but it's a good suggestion to add the .html, will do that in a follow-up PR

Copy link
Collaborator

@knixeur knixeur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been following this, all looks good with latest changes, great work!

@raboof raboof merged commit edfac07 into master Aug 5, 2019
@raboof raboof deleted the liveDocs branch August 5, 2019 12:19
@raboof raboof added the docs label Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants