atuttle / atom-language-cfml

:space_invader: A CFML Language for the Atom Editor
38 stars 24 forks source link

Syntax highlighting inside of strings #53

Closed tjbenton closed 8 years ago

tjbenton commented 9 years ago

Currently It doesn't highlight variables or functions used inside of strings. I looked in the dev tools and there's no way to target them with a stylesheet because they're wrapped in span or anything.

screen shot 2015-08-12 at 11 30 20 am

In the screen shot #_.detail# should be highlighted at the end of that string.

elpete commented 9 years ago

The best thing to do, if you can, is to write a failing test case for the issue you are seeing. You can look at our own spec folder for this and also on the Atom documentation. This will help those maintaining this package know precisely what is broken.

elpete commented 9 years ago

Hey, @tjbenton. I've just pushed a pull request that should fix the issue you are describing. Would you mind testing it out before we merge it in? You can find the instructions on beta testing it on the pull request, #66. Thanks!

tjbenton commented 9 years ago

Yeah I'll take a look at lunch today

tjbenton commented 9 years ago

I had to run apm rm language-cfml then apm link to get it to link properly, the first time I got an error.

Thank you for fixing this it's been driving me nuts!! I really appreciate it!

It looks correct for most of the strings cases. I went ahead and put together a few test cases of different things I've come across in development, some of could probably go under a different pull request. Below is the screenshot of what's actually happening with the syntax highlighting and below the screenshot is the same code I used to help you with your testing.

screen shot 2015-12-01 at 1 52 27 pm

You can see most of the use cases that I mentioned in the comments are being highlighted correctly with the markup syntax highlighter.

<!--- Looks good correct. The only issue I can see happening 
is the that only the first part of the variable get's wrapped in a span, but 
the nested variables don't. Also only the second period(`.`) is highlighted and 
it's being highlighted under a `js` class --->
<link rel="stylesheet" href="#request.site.css#pages/details.min.css">
<meta itemprop="priceCurrency" content="#variables.site.country#"> 
<span class="c-payment-type c-payment-type--#card.name#"></span> 

<!--- perfect! --->
<div class="c-tabs__tab js-sticky <cfif i eq 1>is-active</cfif>"></div> 

<cfloop from="1" to="#variables.cc.accepted.count#" index="i">  <!--- correct --->  
  <!--- cf comments do not render correctly inside of a tag --->
  <input class="js-exsisting-cards__radio" type="radio" name="credit_card_number" 
         value="#i#" <!--- #i# should be hightlighted ---> 
         <cfif i eq -1>checked</cfif>>

  <!--- depending on the order of the cf tag you might see different syntax highlighting --->
  <input class="js-exsisting-cards__radio" type="radio" name="credit_card_number" <cfif i eq -1>checked</cfif> value="#i#"> 

  <!--- commenting out a attribute inside of html tag breaks the rest of the attributes in the tag  --->
  <input <!--- class="js-exsisting-cards__radio" ---> type="radio" name="credit_card_number" <cfif i eq -1>checked</cfif> value="#i#"> 
</cfloop>
<script>
  <cfif variables.awesomesauce>
    document.addEventListener('awesome', function() {
      if ((e.detail || {}).sauce) do_something_sweet(e.detail.sauce)
    })
  </cfif>

  <!--- 
    cf tags don't get styled correctly in `script` tags and
    end up messing up syntax highlighting for the rest of the document
  --->
  $existing_cards
    .on('change', '.js-add-payment-trigger', function() {
      remove_exsiting_validation();
      $new_payment_methods
        .toggleClass('is-open')
    })
</script>

Thank you for taking a look at this!

Let me know if you need any specific environment information

elpete commented 9 years ago

Thanks for the examples. I found a fix for all but the js portion, but want to get tests written first. I'll let you know when it is updated so you can confirm. Thanks.

elpete commented 9 years ago

An update to the branch has been pushed up. A few things:

  1. No fixes to js highlighting yet.
  2. The span you reference at the top is because request is tokenized as a scope. This is intentional. You theoretically could target cfml scopes in a syntax theme.

I believe all of the other issues you raised are fixed. Would you mind checking again? Thanks.

tjbenton commented 8 years ago

The recent changes are working perfectly for me.

It would be easy to scope the delimiter for the . that are wrapped in a span with that class since it is scoped under the cf class. However it doesn't look like the first delimiter after variables isn't wrapped in a span with a class of delimiter.

The only other issue I saw was that attributes that didn't have quotes aren't getting parsed at all. I know it's possible with HTML and some people may right their code without quotes. If it's not a simple fix, then I wouldn't let it hold you up on merging these awesome changes because I don't think having attributes without quotes is very common I just remembered it was possible.

screen shot 2015-12-16 at 9 53 43 am

<!--- 
single and double quotes just needs to have that first delimiter wrapped in 
a span line the other delimiters in the string
--->
<span class='#variables.detail.skuOptions.id[i]#'></span> 
<span class="#variables.detail.skuOptions.id[i]#"></span>

<!--- 
in html you don't need to have quotes on attributes and 
it seems like they aren't getting parsed. It looks like atom is 
highlighting them like this 
``
<span class="string unquoted html">#variables.detail.skuOptions.id[i]#</span>
``
--->
<span class=#variables.detail.skuOptions.id[i]#></span>
elpete commented 8 years ago

Good point on both of those. It looks like wrapping the first delimiter after a scope name is an issue in all the syntaxes. I'll open up a separate issue for that.

As for the unquoted string, I've added a failing test case in pull request #66. I'll also open up a separate issue for that one.

Thanks for all your help testing these fixes! If you're okay with it, I'll close this issue now in favor of the two more specific ones mentioned above.

tjbenton commented 8 years ago

That sounds good to me, are you going to open a pull request for issues that happen inside of <script> tags?

elpete commented 8 years ago

Ah...good reminder. I'll open an issue since I don't know how to solve that one yet.