Skip to content

Conversation

@olaf-k
Copy link

@olaf-k olaf-k commented Jun 17, 2014

Simple grammar update to allow for /* */ multiline comments.

On top of this change I'd challenge the need to keep the value property of comment nodes is the parsed tree since we don't do anything with it afterwards (this would make the rules a tad simpler.) @b-laporte what do you think?

@ymeine
Copy link
Contributor

ymeine commented Jun 17, 2014

If you want later on to be able to do powerful things with the AST, notably for editing purposes (and especially formatting), or simply to make use of the content of the comment, I would keep the value.

@benouat
Copy link
Member

benouat commented Jun 17, 2014

Myself i would even remove all javascript comments (both // and /* */) to be usable from within a template statement :)
It's again mixing html and javascript syntax which add more not needed complexity

# template hello(name)
  // this is the header  <- this does not make sense to me
  <h1>hello {world} !</h1>
  /* 
    This is a comment
    block  <-  this does not make sense either to me */
# /template

Inside a template block, as we are supposed to write html compliant code and/or only statements, I would simply propose to have either

  • standard html comments (that would be generated as part of the template output)
    <!-- this is my comment, that can be multiline -->
  • or a dedicated new statement for that like (that would be converted to html comments generated in the output also)
    {# This is a comment}

@olaf-k
Copy link
Author

olaf-k commented Jun 17, 2014

I don't see any complexity and am obviously against this kind of restriction.
@ymeine yes - it's just that we don't use that today. but it's not a big deal.

@ymeine
Copy link
Contributor

ymeine commented Jun 17, 2014

The first question regarding that @benouat is: do we need to keep some comments in the output? And on the other hand: do we want to be able to put some comments without affecting runtime? If yes for both, I would maintain two kinds of comment syntax: one which keeps the comment in the output (remember how the output is, this might not make sense), and one which does not.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c0a72cb on olaf-k:multicomments into 990cdfa on ariatemplates:master.

@piuccio
Copy link
Contributor

piuccio commented Jun 17, 2014

Nobody cares, but in JSP there are two types of comments

<!-- This one generates an actual comment in the DOM -->

<%-- This one simply disappears, because you don't want to waste bytes or expose information to the user -->

Include the latter and you'll make the life easier for those poor souls that convert JSPs into templates, not that it's very difficult...

@benouat
Copy link
Member

benouat commented Jun 17, 2014

Thank you @piuccio !
You have either the html one, because jsp are meant to produce html ...
Or the one provided by jsp as a jsp statement.

Exactly what I described and propose 😃 !!

@olaf-k
Copy link
Author

olaf-k commented Jun 17, 2014

Supporting <%-- -->would not be difficult either.

As for html comments, HashSpace doesn't keep them in the DOM.

@benouat
Copy link
Member

benouat commented Jun 17, 2014

I was not thinking about implementing the jsp one ! 😆

I was more thinking about providing a new comment statement that inherit from our specific statement grammar like {# This is a comment} or {-- This is a comment --}

PS: I am also kinda fan of Lisp, could we also have the Lisp comment syntax for free ? 😄 👍

; this is a lisp comment

More seriously, for the html one, hashspace should keep them, that is a real bug for sure

@olaf-k
Copy link
Author

olaf-k commented Jun 17, 2014

I was actually replying to @piuccio - you posted your answer in between :) If it makes it easier for people to import their JSP, handling these comments is an easy task. Also I'm favorable to flexibility: I naturally use // and /* */ when writing templates but I'm not forcing anyone to use them.

@marclaval
Copy link
Contributor

I would agree with @benouat and follow the JSP approach reminded by @piuccio, which means:
<!-- Native HTML comments generates an actual comment in the DOM -->
{-- This is a comment statement with a specific syntax, it is ignored by the compiler --}
As a consequence, // and /* */ should be removed.

@olaf-k
Copy link
Author

olaf-k commented Jun 17, 2014

I'm writing Hashspace templates, not simple html nor jsp and disagree.

@ymeine
Copy link
Contributor

ymeine commented Jun 17, 2014

I'm lost. Really.

hashspace has a a syntax for comments: // and /**/. It is considered as comments, at first level, which means something intended only for the developer, not the runtime.

Do we want to be able to comment our templates like this? First question.

HTML has another syntax for comments, which generates something in the DOM, so it has an impact on runtime. This is pretty standard.

Do we want to be able to generate comments in the DOM? Second question.

@benouat You said you would like to remove the hashspace syntax. How could I put a comment which does not go into the DOM in this case?

And secondly, if we support the DOM comment nodes, we should use the HTML syntax for sure, except if a specific statement would add a value.

True that in pure HTML we don't have the choice, every comment goes to the DOM.

@benouat
Copy link
Member

benouat commented Jun 17, 2014

@ymeine

Today, hashspace template syntax is really close to HTML, as in most other templating languages.
To me it should remains like that.

I did not know, as of today, that we were supporting // comment. to me this is wrong.

So my proposal is

  • We have to support real HTML comment inside template syntax <!-- this will be generated in the dom -->
  • We should introduce a new Comment statement {-- this is a comment --} that purpose would be only documentation as they should not even be parsed by the compiler, or maybe syntactically the parser recognize them, but don't do anything with them

@ymeine
Copy link
Contributor

ymeine commented Jun 17, 2014

I agree to have the HTML comment, as long as we consider that we should not simply restrict the HTML capabilities let's say.

But for the statement, in fact we have the equivalent: // and /* */.

Of course, this should not be taken into account by the runtime. For the parsing, it has to be known, but also retained in some use cases, as I said for editing purposes for instance.

So finally we kind of agree.

What @olaf-k seems to be opposed to is to keep the DOM comment nodes. Am I wrong?

@benouat
Copy link
Member

benouat commented Jun 17, 2014

@ymeine my specific point being that I would not like to impose // and /* */. They belong to javascript, and nowhere inside # template we rely on javascript syntax.

This is why kinda disagree. I don't consider them as equivalent.

@ymeine
Copy link
Contributor

ymeine commented Jun 17, 2014

@benouat No, the syntax has been completely borrowed from JavaScript, but this is really a hashspace comment, the equivalent as having the statement you suggest.

Comments have usually had a syntax a bit far from the rest of the language they are used in, for a good reason: distinguish them easily (both from the user point of view and the parsing one).

But I'm fine with having a comment syntax close to other statements, as in HTML, since we are in a case where we mix imperative code and output content: expanding the syntax on one side might reduce the one on the other side, forcing to escape things and so on.

@olaf-k
Copy link
Author

olaf-k commented Jun 17, 2014

Again, templates are written in the Hashspace DSL that is neither HTML nor JS.

While I agree we should decide whether html comments should end up in the DOM (and I guess they should), // and /* */ are widely recognized as comment delimiters (including in AT/Atlas.) This has never been a problem before and no one forces you to use them.

this is ending up as a never-ending thread like #92 because some of you believe your way of coding is the way of coding and alternatives should not exist. I really am uncomfortable with that.

@benouat
Copy link
Member

benouat commented Jun 17, 2014

// and /* */ are widely recognized as comment delimiters

In a lot of imperative languages: true
In declarative languages: false

Yes, i agree that we disagree on that specific point, and it is somehow related to #92 because it involves the same dilemma: Is our DSL an imperative one or a declarative one ?

And yes, I also feel uncomfortable with that, for the exact same reason as yours, hence the never-ending part 😞 😞 😞

@b-laporte
Copy link
Member

OK - I jump into this interesting discussion (!)

As far as I am concerned the criteria I try to use when thinking about syntax changes are (by order of importance):

    1. Will readers understand without reading the documentation?
    1. Is it easy to type?
    1. Is it editor-friendly? (I must say I only added this one recently - sorry Pawel!)

So personally I am fine with the current comments (i.e. //) as it perfectly matches 1 and 2, and doesn't hurt much 3 (IMO). So logically, if we feel the need of multi-line comments we should go for /* */ - and as such accept this PR.

When I look at the other proposals, the only one that match criterion 1 are the HTML comments - but unfortunately they don't match 2 (!). Besides, I also think that HTML comments should be outputed as DOM comments (even if it is not yet the case - but they are already valid as comments).

In a general manner I find that comments with beginning and ending syntax don't match my criterion 2 - so I wouldn't like to have them as unique type of comments. Besides I think comment characters should be visually different from the rest of the syntax as many people still don't use editors (I agree this argument is a bit boderline but I consider it still valid).

So the last consideration should be about editors. Even if one day we may have some dedicated plugins for popular editors, it is not the case today - so we will have to live with JavaScript or HTML editors for a while. I guess HTML editors will be the most popular as it is easy to put JS in separate files and keep only templates in hsp files. In this case HTML comments can be used (and are already accepted) and JS comments don't create much damage (even though mixing tags and JS comments could lead to errors in smart HTML editors - but HTML comments are still there) and other multiline syntax proposals don't do better IMO.

As a conclusion, I personally would accept this PR. From the beginning hashspace is a DSL that borrows both from JavaScript (for expressions and dependency management) and HTML (for tags and entities) - so using // and /* */ for comments should not be surprising for a reader / developer.

@olaf-k I would still add one more test in your PR to validate that /* is properly escaped with \/* as it should be possible to write JS comments in the output (looking at the code it should be OK, but a test would be better) - thx

@benouat
Copy link
Member

benouat commented Jun 17, 2014

ok, then I still have one small question

why are we bothering ourselves with keeping the statements syntax for all other javascript topics like let, if, else, foreach ??

how come could you think that it is fine for comments and not the other ones?

my answer to this question is quite simple: just because we can't, as { } are reserved keywords in javascript and also used as statement delimiter

you can't write that in a template

# template mytpl(data)
  if (data.expr) {
    <div> ... </div>
  }
# /template

so if we can't do it for other statements, as simple as it sounds, for consistency, we should not do it for comment either (after all comments are statements too).

@benouat
Copy link
Member

benouat commented Jun 17, 2014

To try to clear my mind on the topic, I just spent some time to list how comments are handled in most used languages (I basically used the tag #templating on stackoverflow to get a list of popular templating languages upon the 5 first pages)

Here are the results of my research as a neutral reference
All these one still allow you to use standard html comments <!-- -->

  • php (php)
    // or /* */ but you first need to enter the php world from the html with <?php ?>

    <?php // comment ?>
    <?php # comment ?>
    <?php /* multi line 
      comment */ ?>
  • jsp (java)

    <%-- comment --%>

    or same story as php, where you can use // or /* */ but you need to enter jsp world

    <% // comment %>
    <% /* multiline
      comment */ %>
  • asp (asp)

    <%-- comment --%>
  • django (python)
    comments are manage via django statement

    {% comment "this is a comment" %}
      <p>this will not be outputed {{variable}}</p>
    {% endcomment %}
  • handlebars (js)

    {{! single line comment }}

    or

    {{!-- multiline
      comment --}}
  • mustache (js)

    {{! comment }}

    for both single and multiline comments

  • liquid (used by jekyll) (ruby)
    same as django

    {% comment %} this will not be outputed {% endcomment %}
  • dustjs (js)

    {! comment !}
    
    {! multiline
      comment !}
    
  • flask / jinja (python)

    {# comment #}
    {# multiline
      comment #}

In my list the only 2 ones that are using directly // or /* */or even / are not relying on html

  • haml (proprietary compiled to ruby)
    html comments

    /
      %p This doesn't render...
      %div
        %h1 Because it's commented out!

    haml comments

    %p foo
    -# This is a comment
    %p bar
  • jade (proprietary compiled to js)

    // this is a comment (will be converted to <!-- this is a comment -->)
    div.foo(data-id='foo')
      //- this is a source only comment
      p some text here

@Samuda
Copy link

Samuda commented Jun 18, 2014

Hi,
Is not overkill to determine the best way of commenting, when people are not commenting their code and when they do it is for saving code that is not used anymore but can be reused later.
That said.
As a future user of hashspace, I will better go in the Benouat direction having a syntax that is close to the language itself. It is avoiding convusion, I think could really give a split between part of the framework.

@b-laporte
Copy link
Member

@benouat thanks for the all the samples - but as far as I can see none of the proposals give a simple single-line comment such as // - which is for me the most useful type of comments.

Regarding your first comment (i.e. why don't we use JS syntax everywhere) the reason is that we would need to put too many keywords in the grammar, and have to propose escape options for each of them. Besides statements would be easily visually mixed with normal text - so of course this doesn't fly, But IMO this argument doesn't apply to the current propoasl - i.e. // and /* */ - that is well-know by everyone and that cannot be easily mixed with normal text.

@benouat
Copy link
Member

benouat commented Jun 18, 2014

that is well-know by everyone and that cannot be easily mixed with normal text.

@b-laporte human brains are good at not mixing them, computers not at all !

on top of that, in my list, all of them propose the single line comment (everywhere i used comment for single line and multiline comment for multiline ! 😄

@b-laporte
Copy link
Member

Well what I meant, is that the "single line comments" of the other languages are as as verbose as multi-line comments (not to mention that it was not obvious to me that some were actually meant to be comments) - so what's the point? If we think comments are important, it should be extremely simple to write them - otherwise as @Samuda said, it will be a very good reason to not comment templates..

@olaf-k
Copy link
Author

olaf-k commented Jun 18, 2014

@benouat a few things:

  • about the "declarative vs imperative" aspect that seems to bother you: Hashspace is not declarative by design: it still depends on context (data model) and is not immune to side effects. I guess you could use it in a declarative way as much as possible the same way you could use JS to do functional programming but those are not intrinsic properties of the languages.
  • this being said, I don't see what it has to do with characters used for comments.
  • we may both feel uncomfortable but both PR are about introducing a familiar syntax to some programmers while leaving you free to not use them and in both cases these optional changes are dismissed by people who would not use them.

@b-laporte sure - test updated.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 897f7b7 on olaf-k:multicomments into 14bdf0b on ariatemplates:master.

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.

8 participants