babel-code-frame: add options for linesBefore, linesAfter #4561
Conversation
hzoo
commented
Sep 24, 2016
hzoo
commented
| Q | A |
|---|---|
| Bug fix? | no |
| Breaking change? | no |
| New feature? | yes |
| Deprecations? | no |
| Spec compliancy? | no |
| Tests added/pass? | yes |
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |
hzoo
added
the
PR: New Feature 🚀
|
@lydell is |
Current coverage is 88.47% (diff: 100%)@@ master #4561 diff @@
==========================================
Files 192 192
Lines 13491 13493 +2
Methods 1412 1412
Messages 0 0
Branches 3126 3128 +2
==========================================
+ Hits 11936 11938 +2
Misses 1555 1555
Partials 0 0
|
|
From js-tokens’ readme:
In the initial PR where js-tokens was introduced (back in the Babel 5 days), is-keyword-js actually was used, but was quickly removed since esutils already was a dependency of Babel 5. |
|
Also, don’t forget to add the new options to the table in babel-code-frame’s readme :) |
|
Oh ok so we need it - weird I did add the readme but it wasn't pushed😃 |
|
@lydell ok just going to use a list of keywords - also it was because I was modifying node_modules/readme for some reason haha |
| @@ -38,3 +38,5 @@ If the column number is not known, you may pass `null` instead. | |||
| name | type | default | description | |||
| -----------------------|----------|-----------------|------------------------------------------------------ | |||
| highlightCode | boolean | `false` | Syntax highlight the code as JavaScript for terminals | |||
| linesAbove | number | 2 | The number of lines to show above the error | |||
| linesBelow | number | 3 | The number of lines to show below the error | |||
There was a problem hiding this comment.
At least in this github diff, the columns don’t seem to line up :)
| ]; | ||
|
|
||
| function isKeyword(str) { | ||
| return KEYWORDS.indexOf(str) > -1; |
There was a problem hiding this comment.
Would it make sense to let KEYWORDS be a Set and do KEYWORDS.has(str)?
There was a problem hiding this comment.
We're not using transform-runtime or node 4 so just doing this or we can hasOwnProperty it
hzoo
force-pushed
the
code-frame-opts
branch
2 times, most recently
from
cdec3b5
to
e3b1ea4
Compare
hzoo
force-pushed
the
code-frame-opts
branch
from
e3b1ea4
to
27711d1
Compare
| "while": true, | ||
| "with": true, | ||
| "yield": true | ||
| }; |
There was a problem hiding this comment.
we also use esutils in in other places to check for keywords. If we now want to manage keywords ourself should we maybe but them in a central place, so all packages can use them?
There was a problem hiding this comment.
Was going to remove the deps for this package so other tools can use easily, what do you think? Or maybe it's not that big of a deal and we should leave it
| "while": true, | ||
| "with": true, | ||
| "yield": true | ||
| }; |
There was a problem hiding this comment.
I don't have a strong opinion about using or not using esutils, but I think it would be nice to have a single source of truth for keywords throughout all the babel packages.
There was a problem hiding this comment.
I'l just leave esutils in for now and we can decide that later
|
Ok it just adds the option |
|
@lydell good? going to merge this first |
|
Yes! 👍 |
* babel-code-frame: add options for linesBefore, linesAfter * add example, use list of keywords * a [skip ci] * Update index.js
lock
bot
added
the
outdated
