Skip to content
This repository has been archived by the owner on Sep 24, 2021. It is now read-only.

Add standard snippets for html import and polymer element definitions #36

Closed
wants to merge 1 commit into from

Conversation

TimvdLippe
Copy link
Contributor

@TimvdLippe TimvdLippe commented Nov 29, 2016

This adds the most important snippets of https://github.com/robdodson/Atom-PolymerSnippets/tree/master/snippets into the editor-service. Typing <html-import>, <polymer-element> or <tdd-test> will now show these autocompletions in Atom.

Fixes Polymer/atom-plugin#11

CC @robdodson

  • CHANGELOG.md has been updated

This change is Reviewable

@robdodson
Copy link

LGTM!

@abdonrd
Copy link

abdonrd commented Nov 30, 2016

Maybe rename <polymer-element>/pe to <dom-module>/dm?

@TimvdLippe
Copy link
Contributor Author

@abdonrd Since the autocompleter in Atom/Vscode works for tags (see https://github.com/Polymer/atom-plugin/blob/master/src/auto-completer.ts#L68) and <polymer-element> is more descriptive, I would keep it this way.

@rictic
Copy link
Contributor

rictic commented Dec 13, 2016

Ideally these would be enhanced autocompletions for real existing elements, as that's how they'll appear in the UI, so I think dom-module may be better.

I want to get a sense of what our options will be with textmate-style snippets though, so let's wait until the next VSCode release, should be dropping tomorrow: microsoft/vscode#15099

Copy link
Contributor

@rictic rictic left a comment

Choose a reason for hiding this comment

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

VSCode now has an implementation of the textmate snippet syntax, so we may soon be able to unify on only providing expandToSnippet. Just waiting on them to add it to the protocol.

},
{
'tagname': 'polymer-element',
'description': 'Template definition of a Polymer 2 element',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should drop this for now, as Polymer 2.0 isn't out yet, and most users initially are likely going to want to write hybrid elements.

'description': 'Template definition of an HTML import.',
'prefix': 'hi',
'expandTo': '<link rel="import" href="bower_components/.html">',
'expandToSnippet': '<link rel="import" href="${1:bower_components}/${2}/${2}.html">'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is tricky. If it's an app then they'll want bower_components/ or similar, but if it's a standalone element it should be ../.

We really need a signal that lets us tell whether some code is an app or a standalone component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes agreed, shall I extract an issue for that in the CLI? I think adding a line to the bower.json stating the type is the cleanest solution?

<head>
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>$1</title>
<script src="\${2:bower_components}/webcomponentsjs/webcomponents-lite.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a lot of this could be based off the element tagname. i.e.

<title>${1:my-element}</title>
...
<link rel="import" href="./$1.html">
...
<test-fixture id="basic">
  <template>
    <$1></$1>
  </template>
</test-fixture>
...
<script>
  suite('$1 tests', function() {
    ...
  });
</script>

@TimvdLippe
Copy link
Contributor Author

It seems like the protocol has landed support for snippets: https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request

Also we should consequently update the atom-plugin to make use of the protocol instead of the custom server.

@TimvdLippe
Copy link
Contributor Author

@rictic What would be the next steps to get this PR going again? Missing it in my own workflow 😄

@TimvdLippe
Copy link
Contributor Author

@rictic What's the status of this PR? Should I finish this still?

@rictic
Copy link
Contributor

rictic commented Dec 5, 2017

So, good news, bad news.

Bad news is that rebasing will involve significant changes just because of changes in the project structure.

Good news is that in the new structure I think we have enough info to do a really good job at this. Can look at the lint rules in the project config (see diagnostics.ts for how to do this) to see if it's a Polymer 1, Polymer 2 Hybrid, or Polymer 2 app, to suggest the right autocompletion for a new element template.

@TimvdLippe
Copy link
Contributor Author

I will start on a new branch to make that easier. Will work on it this weekend 😄

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.

4 participants