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

Generate names array for sourcemaps #113

Merged
merged 1 commit into from
Jul 4, 2013
Merged

Generate names array for sourcemaps #113

merged 1 commit into from
Jul 4, 2013

Conversation

tivac
Copy link
Contributor

@tivac tivac commented Jul 1, 2013

The Mozilla source-map library already supported the names array, it just wasn't getting passed the right param. I'm not checking the type of the node because only nodes that we care about should have a name property. If that's not kosher it would be straightforward to check that node.type === Syntax.Identifier first before passing it into new SourceNode(), up to you guys!

Tried to follow test format closely. If I didn't get it quite right I'd love feedback!

@tivac tivac mentioned this pull request Jul 1, 2013
@Constellation
Copy link
Member

Thanks for your request. Merging.

Constellation added a commit that referenced this pull request Jul 4, 2013
Generate names array for sourcemaps
@Constellation Constellation merged commit 46909e0 into estools:master Jul 4, 2013
@tivac tivac deleted the source-map-names branch July 4, 2013 03:42
@nopnop
Copy link

nopnop commented Jul 12, 2013

@tivac @Constellation
The source-map names array is useful to get the original source name, not the mangled name. However, the name property used here to generate the names array is certainly mangled (by esmangle for instance: estools/esmangle#11)

May I suggest to reopen this issue, and to consider this one: estools/esmangle#85

@Constellation
Copy link
Member

Yeah, right. I'll reopen this issue.

edit: I'll open new issue.

@tivac
Copy link
Contributor Author

tivac commented Jul 14, 2013

Well that's embarassing, I'll take another run at this before too long hopefully.
On 7/14/2013 3:48:18 PM, Yusuke Suzuki [email protected] wrote:
Yeah, right. I'll reopen this issue.

Reply to this email directly or view it on GitHub [https://github.com//pull/113#issuecomment-20945480].

@nopnop
Copy link

nopnop commented Jul 16, 2013

Many thanks @tivac. This is an useful part for the near future in a great tool.

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