Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

[FunctionSearch] name functions based on their var#2404

Merged
wldcordeiro merged 4 commits into
firefox-devtools:masterfrom
ruturajv:bug-2323-function-search-anonymous-functions
Mar 22, 2017
Merged

[FunctionSearch] name functions based on their var#2404
wldcordeiro merged 4 commits into
firefox-devtools:masterfrom
ruturajv:bug-2323-function-search-anonymous-functions

Conversation

@ruturajv

Copy link
Copy Markdown
Contributor

Associated Issue: #2323

  • Suggested fix for function definitions for object members

Its a WIP patch :) please bear.

@codecov-io

codecov-io commented Mar 17, 2017

Copy link
Copy Markdown

Codecov Report

Merging #2404 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #2404   +/-   ##
======================================
  Coverage       0%      0%           
======================================
  Files          77      77           
  Lines        2485    2485           
======================================
  Misses       2485    2485

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 fad4b80...e9aca65. Read the comment docs.

Comment thread src/utils/parser/utils.js
if (parent.right && parent.right.type == "FunctionExpression") {
return parent.left.property.name;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃

Comment thread src/utils/tests/parser.js Outdated
"count",
"counter",
"sum",
// "property",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the commented line?

Comment thread src/utils/parser/utils.js Outdated
return parent.id.name;
}

if (parent.right && parent.right.type == "ArrowFunctionExpression") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be able to avoid the two conditions by using the isFunction helper I wrote.

if (parent.right && isFunction(parent.right) {
  return parent.left.property.name;
}

@wldcordeiro

Copy link
Copy Markdown
Contributor

@ruturajv a couple comments, what do you mean by "pending variable searches"?

@ruturajv

Copy link
Copy Markdown
Contributor Author

@wldcordeiro - The original issue also wanted the object's members to be listed in search for "variables" ie for Obj.member = "abc", even member should be listed in as a variable. That is pending.

@wldcordeiro

Copy link
Copy Markdown
Contributor

The original issue also wanted the object's members to be listed in search for "variables" ie for Obj.member = "abc", even member should be listed in as a variable. That is pending.

@ruturajv

They already are though. There was an unresolved question about whether an object member should appear in both variables and functions.

See this test.

https://github.com/devtools-html/debugger.html/blob/master/src/utils/tests/parser.js#L112
https://github.com/devtools-html/debugger.html/blob/master/src/utils/tests/parser.js#L217

@ruturajv ruturajv changed the title WIP: Anonymous functions done, pending: variables search [FunctionSearch] name functions based on their var Mar 18, 2017
Comment thread src/utils/parser/utils.js Outdated
return parent.left.property.name;
}

if (parent.right && isFunction(parent.right)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if condition is the same as the one above, it can be removed, otherwise this looks great.

@wldcordeiro wldcordeiro merged commit 7db1e9f into firefox-devtools:master Mar 22, 2017
@ruturajv

Copy link
Copy Markdown
Contributor Author

thanks @wldcordeiro

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants