Skip to content
This repository has been archived by the owner on Aug 8, 2020. It is now read-only.

Snippets improvements #5

Closed
wants to merge 5 commits into from
Closed

Conversation

kipit
Copy link
Contributor

@kipit kipit commented Feb 24, 2016

Small improvements introducing :

  • module name guessing from default function name
  • optional title (simply remove the title to skip title use)

This PR also contain a file renaming (cmd+D are you there?).

@kipit kipit mentioned this pull request Feb 24, 2016

test('${3:title}', t => {
test(${3/(.+)|.*/?1:'/}${3:title}${3/(.+)|.*/?1:', /}t => {
Copy link
Member

Choose a reason for hiding this comment

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

Is the |.* part really needed? If yes, what purpose does it have? I might be missing something obvious since it's late here, but seems to be working fine without.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hell no! Tiredness make me write unecessary complicated patterns. As you will see in the next commit, the final regex is wayyyy simpler. Thank you.

@sindresorhus
Copy link
Member

I didn't find a way to make the second placeholder inside the quote. If I wrote, instead import $1 from '${2:${1/([A-Z])/-\l$1/g}}';, the cursor fall at the begining of the module name and the module name is not selected.

I think you made the right choice. Just open an issue when this PR is merged about finding a solution and maybe someone will come along and know a better way ;)

@sindresorhus
Copy link
Member

This is really cool. I like it a lot :)

@kipit
Copy link
Contributor Author

kipit commented Mar 5, 2016

I’ve also corrected a typo in the test-serial snippet.

@kipit
Copy link
Contributor Author

kipit commented Mar 5, 2016

I think you made the right choice. Just open an issue when this PR is merged about finding a solution and maybe someone will come along and know a better way ;)

Let's first notify them: sublimehq/sublime_text#1132 ;-)

@sindresorhus
Copy link
Member

Let's first notify them: sublimehq/sublime_text#1132 ;-)

That's not an official issue tracker. You'd probably have better luck in the official forum: https://forum.sublimetext.com/

@sindresorhus
Copy link
Member

Awesome! Thanks for working on this @kipit :)

@sindresorhus
Copy link
Member

I opened an issue here too so we can track it better: #6

@sindresorhus
Copy link
Member

@kipit Would you be interested in working on #1? Should be easy to port the Atom ones: https://github.com/sindresorhus/atom-ava/blob/master/snippets/assertions.json

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants