Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

Add callers: and called-by: filters for the Python plugin. #399

Merged
merged 2 commits into from
Mar 20, 2015

Conversation

Osmose
Copy link
Contributor

@Osmose Osmose commented Mar 8, 2015

Because the return order of ast.walk is not guaranteed to mean anything, I needed to be able to control the order that we walk through the tree if I wanted to correctly assign function calls to the right caller. The first commit refactors the Python plugin to move all the searching for needles to a NodeVisitor subclass. The second commit adds the new filters and some tests for them.

I spent some time trying to see if I could make a NodeVisitor subclass that returned a generator of needles, but I couldn't figure anything out, and I didn't want to copy over the NodeVisitor code and tweak it just for that. Oh well.

@Osmose
Copy link
Contributor Author

Osmose commented Mar 8, 2015

@erikrose r?

@Osmose Osmose force-pushed the python-callers branch 2 times, most recently from fc48086 to 2c849a7 Compare March 10, 2015 20:14
@erikrose
Copy link
Contributor

Sorry this took so long.

So could we later use your import mapping to support fully qualified caller searches, like +function:urllib.parse()?

def is_interesting(self):
return is_interesting(self.path)

def needles_by_line(self):
self.analyze_tokens()
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of making analyze_tokens() return its results and changing this statement to assign them to self.node_start_table? I generally like to bubble instance attr assignment up as far as possible to make clear the side effects. Passing the entire file_to_index into the visitor smells a little to me, too, but I can't think of a better idea right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

@erikrose
Copy link
Contributor

That's all I have to say about that. All in all, looks good to me!

@Osmose
Copy link
Contributor Author

Osmose commented Mar 12, 2015

So could we later use your import mapping to support fully qualified caller searches, like +function:urllib.parse()?

I believe so. You know how it traces the imports back until they're not imported anymore? I bet we could have a function that yields each of those jumps and then index each of them as needles, similar to how I had it indexing multiple needles for qualnames before switching to use QUALIFIED_NEEDLE.

@Osmose
Copy link
Contributor Author

Osmose commented Mar 13, 2015

Updated. Kept called-by for now until you reply.

@erikrose
Copy link
Contributor

So let's (1) kill off called-by and then merge. Then the next thing should be to force the utf-8 offsets back into Unicode and write some tests to make sure the offsets from tokenize and ast agree in the face of unicode. That doesn't have to be part of this PR since, if it's broken here, it was already broken before.

Michael Kelly added 2 commits March 20, 2015 10:56
@Osmose Osmose merged commit 4f52212 into mozilla:es Mar 20, 2015
@Osmose Osmose deleted the python-callers branch March 20, 2015 18:15
@Osmose
Copy link
Contributor Author

Osmose commented Mar 20, 2015

Removed and merged. (Sorry if you wanted to take a last look at this, I interpreted you as giving this an r+wc)

@erikrose
Copy link
Contributor

Nope, you did the right thing!

jbradberry added a commit to jbradberry/dxr that referenced this pull request Oct 24, 2015
Support had already been removed, this just cleans up the last
vestiges.  See mozilla#399.
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.

2 participants