Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add JSP snippets #66

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

MoritzKn
Copy link
Contributor

@MoritzKn
Copy link
Contributor Author

MoritzKn commented Sep 30, 2016

After using this for myself for some time I discovered a usability problem with the JSTL Tag Snippets:
For example

<c:if<tab>

results in

<c:<c:if test="${true}">

</c:if>

because if is a snippet prefix.
I don't know what would be a good alternative... Maybe <c:if instead of if, but that's pretty verbose.

@MoritzKn
Copy link
Contributor Author

MoritzKn commented Sep 30, 2016

Or is it possible to use a regex like (?:^|[^:])if as prefix?

The closing tag was not closing.
@MoritzKn
Copy link
Contributor Author

I figured it would be nice if "tag snippets" would only work outside of tags. So that this:

<div out[tab]>
</div>

wouldn't result in this:

<div <c:out value="${}"/>>
</div>

@winstliu
Copy link
Contributor

Hmm. c:if should definitely take priority over just if. I wonder where that matching occurs.

@MoritzKn
Copy link
Contributor Author

@50Wliu I've done some research on that <c:if vs if problem:
The snippets package does -- probably for performance reasons -- some complicated stuff, but basically it just checks if the text behind the cursor is equal to the prefix of a snippet in scope and if that's the case, it replaces the prefix with the snippet text.

Now I could do two things:

  • Add a second version of each "tag snippets" whose prefix starts with <c:
  • Add a second version of each "tag snippets" with out any prefix. That's what the language-html package has done. This will remove the snippets inside tags.

I prefer the first method, but I don't like that this means I have to duplicate all the code.

@MoritzKn
Copy link
Contributor Author

I fixed this problem in the last commit. It's not perfect because it adds some redundancy, but I think it's the best we can get. Correct me if I'm wrong.

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

Successfully merging this pull request may close these issues.

2 participants