Skip to content

Conversation

@macintacos
Copy link
Contributor

Description

Closes #108.

Added menus and commands that made sense given what we have in the Markdown All in One extension. Surprisingly enough, there aren't really that many commands that the Markdown All in One extension provides, but I don't think we want to layer on a more extensions unless we have to.

One thing that came up when I was trying to put these together was that it'd be sweet if we could expand code snippets directly via vscode-which-key when when trying functionality that wasn't there, such as adding the SPC m x k mapping that's in the Spacemacs Markdown layer for example. Without a way to expand code snippets directly, I didn't think there was any appropriate way to add that. Potential feature request?

I referenced the Markdown layer for Spacemacs when trying to come up with where to put these mappings.

Checklist

CHANGELOG:

  • Updated
  • I will update it after this PR has been discussed
  • No need to update

@stevenguh
Copy link
Member

That extension also shipped with some default shortcut (although limited to markdown doc), are those interfering in anyway?

@macintacos
Copy link
Contributor Author

Not from what I can tell, this is just an abstraction on top of it (I didn't disable any default keybindings and they all seem to work just fine)

@macintacos
Copy link
Contributor Author

@stevenguh RFAL

@stevenguh
Copy link
Member

That looks good to me. What you think, @marcoieni?

@marcoieni
Copy link
Member

This is great, I will see myself using it daily, thank you! Just a few notes:

  1. The file uses tabs, while you used spaces, so if you don't mind you should change it (you are not the first one that does this error, maybe in the future we should talk about using spaces instead)
  2. I don't like the "Buffer Command" name. It is not really meaningful to me. I propose you to have:
    • spc e to export to html
    • spc p for the two preview commands, so you will have spc p p and spc p P

@marcoieni
Copy link
Member

Ignore point 2. I saw that they are the same commands for spacemacs. Sorry.

@marcoieni
Copy link
Member

After you've replaced spaces with tabs, you can update the changelog and it's ok to merge this.

@macintacos
Copy link
Contributor Author

Hey all, apologies, I'll have this fixed up in a few days (currently on vacation, not near a computer).

@stevenguh
Copy link
Member

stevenguh commented Oct 3, 2020

No worries. Thank you for letting us know and enjoy your vacation 😊

@macintacos
Copy link
Contributor Author

@marcoieni indentation converted to tabs. Apologies about that.

My $0.02, in projects that I've set up at my job we set up pre-commit hooks that style code exactly the way that we need so that this isn't a problem that comes up (styling can be whatever you want while developing, but is forced to "conform" when committing) 🙂. In this case, I'd recommend something like Prettier to run when committing, and maybe just make a quick Makefile with a target install that just sets up the hook correctly and installs necessary dependencies. Up to you though, there are many ways to go about setting up something like that.

Please let me know if you need me to modify anything else!

@stevenguh
Copy link
Member

stevenguh commented Oct 4, 2020

Just FYI, the style of the package.json is enforced by npm install(i.e. running npm install will fix the format of package.json). However, call npm install per commit might be too heavy since it also has network call (e.g. fetch packages).

Anyway, setting up prettier or something like that would definitely be a nice enhancement :)

@macintacos
Copy link
Contributor Author

Oh woops, forgot to update the changelog, one sec!

@macintacos
Copy link
Contributor Author

Okay should be good now, lmk if I missed anything!

@marcoieni marcoieni merged commit 2ffc10b into VSpaceCode:master Oct 5, 2020
@marcoieni
Copy link
Member

Thank you so much, this is awesome! 🚀

For the code style discussion I opened #114

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Markdown Major Mode

3 participants