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

Test tags #277

Closed
wants to merge 4 commits into from
Closed

Test tags #277

wants to merge 4 commits into from

Conversation

User4martin
Copy link

The current behaviour of some closing tags is in my opinion "strange"

But never mind that. This test ensures the current behaviour is not accidentally changed.
I assume that changes to this (if desired) would require appropriate version changes, as they would not be compatible.

Behaviour tested (new):

  • any closing %> -%> _%> tag is ignored (silently removed), if it does not have an opening tag.
  • ignored closing tags do slurp space/newlines as they would do, if they had an opening tag.
  • closing %> tag (not tested for -%> _%>) is handled as literal (copied to new source), if it follows an CLOSING literal
    e.g %%> %> will give %> %>

Behaviour tested (already tested in existing test case):

  • closing %> tag (not tested for -%> _%>) is handled as literal (copied to new source), if it follows an opening literal
    e.g <%% %> will give <% %>

There are no test for literals in embedded js code. <%= "<%% foo" %>
(They break the embedded code, as they change this.mode)

@mde
Copy link
Owner

mde commented Jul 29, 2017

I'm not sure I agree that undocumented (and possibly undesirable) behavior has to be preserved as a part of semver conformance. You could potentially treat fixes for this weird behavior as a bugfix.

Do you think we should be testing to preserve existing weird behavior, or should we have a point-by-point discussion about what the desired behavior is?

@RyanZim
Copy link
Collaborator

RyanZim commented Jul 30, 2017

@mde I vote to freeze current behavior in tests. We are such a depended-upon package, we cannot afford to ship "bugfixes" that change behavior, no matter how odd it may be.

However, I do think we should consider starting work on a new major release, that among other things would fix this weird behavior. @User4martin has been a tremendous help, and with that extra help, I think we could get a major out. I can do some work on it, but I am quite busy.

@mde
Copy link
Owner

mde commented Jul 30, 2017

If it were documented, or even major undocumented APIs, I would be more inclined to agree with you. I'm trying to imagine a scenario where someone might depend on "ignored closing tags do slurp space/newlines as they would do, if they had an opening tag." Or maybe a better question might be: when might someone depend on that, and reasonably expect that it never change?

I don 't feel strongly about it, but it could take us quite a while to reach a new major version. If we're going to add these tests, they should probably be explicitly called out as being for functionality that we don't necessarily want to preserve in subsequent versions, so we don't just perpetuate this stuff forever.

@mde
Copy link
Owner

mde commented Jul 30, 2017

Could we label the relevant tests as intended for deprecation?

@User4martin
Copy link
Author

No longer needed, as the behaviour will change in #295

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.

3 participants