DFreds / code-peek-atom

Atom package to peek functions in different files
MIT License
22 stars 10 forks source link

js arrow function -> "Could not find function "fnName" in project." #18

Open jwverzijden opened 7 years ago

jwverzijden commented 7 years ago

trying to peek at an JS arrow function results in the warning "Could not find function "fnName" in project."

const foo = x => x*2
foo( 10 )

peeking at the foo call generates the warning

DFreds commented 7 years ago

Thanks for reporting this. The current regex to find a JavaScript function is: function\s+REPLACE\s(|REPLACE\s(=|:)\sfunction\s(|REPLACE\s([=:])?\s(?\s(([\,\s\w])?)?\s*=>

That matches something like this:

REPLACE(() => { this.foo = foo; }, 1000);

It does not match a const function though. I'll look into improving it to match more. Are there any other examples that do not match that you've found? Thanks again.

jwverzijden commented 7 years ago

the problem is not that the function is a constant. the problem apears to be the parentheses that may be left out if only 1 argument is used something similar happens with destructuring and spread operations

const canBeFound = () => {}
const canNotBeFound = x => {}
const canNotBeFound = ({x,y,z}) => {}
const canNotBeFound = ( ...args ) => {}

functions made by Object.defineProperty( ... ) cant be found either, but i can imagine its a tough one to match with a regex :P

DFreds commented 7 years ago

Thanks for the detailed clarification. I'll work on fixing those cases. Keep me updated if you find anymore. Thanks again!

boazblake commented 7 years ago

I have this issue too! thanks for working on this

jonyeezs commented 7 years ago

const canNotBeFound = x => {}

This is a single specific rule Parentheses are optional when there's only one parameter.

I reckon it'll be easier to manage this with its own regex: REPLACE\s*([=:])?\s*\w+\s*=>

const canNotBeFound = ({x,y,z}) => {} const canNotBeFound = ( ...args ) => {}

For the others, it is tricky. There are even more combinations than that!

var f = ([a, b] = [1, 2], {x: c} = {x: a + b}) => a + b + c;

I recommend for a more loose regex that will cater to any parameter and leave it to the programmer to write correct syntax.

propose regex to replace current es6: REPLACE\s*([=:])?\s*\(?\s*\([^\(\)]*\)\s* =>

DFreds commented 7 years ago

I haven't had a chance to look at this yet, but thank you for providing assistance. I'll let you know if it works!

DFreds commented 7 years ago

@jonyeezs, the proposed regex still doesn't match const REPLACE = x => {}.

I'm trying out this regex: REPLACE\s*([=:])?\s*\(?\s*\(?[^\(\)]*\)?\s*=>

Seems to work (I made the \( and \) matchers optional with a '?'). Can anyone confirm with more examples? Here's my regex link I'm using to test: https://regex101.com/r/qLNVaD/1

jonyeezs commented 7 years ago

I proposed two regex in my previous comment. Both regex needs to be implemented

The first is the case for a single argument (which is what is discussed).

The second is to cater all other permutation.

Hopefully my explanation in my previous post explain why I did that.

Which one were you using to test?

On Sat., 13 May 2017, 6:07 am DFreds, notifications@github.com wrote:

@jonyeezs https://github.com/jonyeezs, the proposed regex still doesn't match const REPLACE = x => {}.

I'm trying out this regex: REPLACE\s([=:])?\s(?\s(?[^()])?\s*=>

Seems to work (I made the ( and ) matchers optional with a '?'). Can anyone confirm with more examples? Here's my regex link I'm using to test: https://regex101.com/r/qLNVaD/1

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/DFreds/code-peek-atom/issues/18#issuecomment-301173460, or mute the thread https://github.com/notifications/unsubscribe-auth/AKXAhnJICEaIbjH-Pjw4kbJ07CRsvx2Iks5r5LvpgaJpZM4L698U .

DFreds commented 7 years ago

I see. So I think it matches fine, but the issue is having it only show that function and nothing else. Currently, Code Peek handles only showing the specific function in the panel by either relying on brackets or tabs to know when the function declaration is complete.

I'm currently going through a refactor based on a suggestion to show the entire file but start at the currently peeked function. That is being worked on in PR #35 for issue #32. Once it is complete and I iron out the last bugs, this will no longer be an issue.

Therefore, I'm going to put this fix on hold until the WIP branch is complete.

jonyeezs commented 7 years ago

Sounds good to me

On Sat., 13 May 2017, 9:38 am DFreds, notifications@github.com wrote:

I see. So I think it matches fine, but the issue is having it only show that function and nothing else. Currently, Code Peek handles only showing the specific function in the panel by either relying on brackets or tabs to know when the function declaration is complete.

I'm currently going through a refactor based on a suggestion to show the entire file but start at the currently peeked function. That is being worked on in #35 https://github.com/DFreds/code-peek-atom/pull/35 and reference issue #32 https://github.com/DFreds/code-peek-atom/issues/32. Once it is complete and I iron out the last bugs, this will no longer be an issue.

Therefore, I'm going to put this fix on hold until the WIP branch is complete.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DFreds/code-peek-atom/issues/18#issuecomment-301208638, or mute the thread https://github.com/notifications/unsubscribe-auth/AKXAhhrRIWvS0e3-Fiaz46Pz2-CiBMkJks5r5O2FgaJpZM4L698U .