This repository has been archived by the owner. It is now read-only.

add startLine option #346

Merged
merged 1 commit into from Feb 10, 2017

Conversation

Projects
None yet
6 participants
@edge
Copy link
Contributor

edge commented Feb 3, 2017

Q A
Bug fix? no
Breaking change? no
New feature? yes
Deprecations? no
Spec compliancy? no
Tests added/pass? yes
Fixed tickets
License MIT

Adding a startLine option allows tool authors to more easily co-opt the babylon API with custom preprocessing.

Our team at RunKit would benefit from this feature.

@edge edge force-pushed the edge:master branch from cf3e9d2 to 2aeeb3b Feb 3, 2017

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 3, 2017

Codecov Report

Merging #346 into master will not impact coverage.

@@           Coverage Diff           @@
##           master     #346   +/-   ##
=======================================
  Coverage   97.65%   97.65%           
=======================================
  Files          20       20           
  Lines        3326     3326           
  Branches      882      882           
=======================================
  Hits         3248     3248           
  Misses         30       30           
  Partials       48       48
Impacted Files Coverage Δ
src/options.js 100% <ø> (ø)
src/tokenizer/state.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 898c4a7...c0e579c. Read the comment docs.

@edge edge force-pushed the edge:master branch from 2aeeb3b to c0e579c Feb 3, 2017

@danez

This comment has been minimized.

Copy link
Member

danez commented Feb 6, 2017

Can you describe a usecase where a tool wants to change the first line to be something different than 1?

@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented Feb 6, 2017

so specifically what's the usecase for runkit? (which is awesome btw)

@Me1000

This comment has been minimized.

Copy link

Me1000 commented Feb 6, 2017

Hey there, I'm from the RunKit team. I've attached an image which should help explain things a bit better.

screen shot 2017-02-06 at 2 17 43 pm

As you can see, cell 2, in this example, starts on line 5. We run each cell through Babel independently. The result is in this example that the generated SourceMap will maps line 1 of cell 2 to line line 1 (rather than line 5 of the notebook). In other words, each cell's first line will always map to line 1.

Our current workaround is to just insert new lines before we parse it. This PR will let us clean some of that up.

@danez

danez approved these changes Feb 7, 2017

Copy link
Member

danez left a comment

I'm fine with that.

@danez danez merged commit f25a2fb into babel:master Feb 10, 2017

3 checks passed

codecov/patch 100% of diff hit (target 97.65%)
Details
codecov/project 97.65% (+0%) compared to 898c4a7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@loganfsmyth

This comment has been minimized.

Copy link
Member

loganfsmyth commented Feb 10, 2017

@Me1000 I'm curious, since you run each section through babel separately, do you not run into issues where Babel has renamed things? Take for example two snippets with

var fn = function(){
  console.log(fn);

}
fn = function(){};
fn();

I'd expect that to fail because Babel renames fn in the first snippet to _fn and the second snippet has no way to know about that.

@Me1000

This comment has been minimized.

Copy link

Me1000 commented Feb 10, 2017

Great question. In short, we're applying our own custom transforms which (among other things) preserve those identifiers.

@loganfsmyth

This comment has been minimized.

Copy link
Member

loganfsmyth commented Feb 10, 2017

Okie doke, cool, just wanted to make sure since compiling separate snippets is not something we officially guarantee to work because of the renaming and such.

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