Adding function information to the /render page by AlonBilman · Pull Request #134 · tmr232/function-graph-overview

AlonBilman · 2025-03-19T14:52:32Z

Hey Tamir!

We've added function info to the render page - it updates only when fetching from GitHub (via URL).
Currently, most of the code is in App.svelte.
Here are some example links to test locally (port 5173):
1
2
3
4
5
6
7
8
We are waiting for your feedback and hopefully it meets your standards.

…ith github repos

added css

tmr232

Hi Alon,

Thanks for your contribution!

I went over it, and there are a few things that need more work.
There's a bit of code to change and move around, and tests that need to be written.

Additionally, I think there are visual-design changes required:

The current color is shared for both the dark and light themes, and only kinda works with either. I think it is better to use different colors for the different themes. This is especially important on the light theme, that currently has very low contrast of text and background.
As this is a programming tool, and we're presenting data, I think a monospaced font will be a better fit.
There is currently a lot of spacing in the display. Between the lines and between the labels and the info. I think it should be tightened.
I think the text should be selectable. There is no reason to prevent selection.
I think it'll be better for the data to be presented under the controls and not on the opposite side
Maybe it's worth it to add the project and filepath as well as the function name?

If you have any disagreement regarding the visual side - please let me know!

Last but not least - please make sure you run the linters (bun lint) and they pass before submitting your code. Note that this is critical, and in most cases I would not review code that does not pass the automated checks.

AlonBilman · 2025-03-20T19:59:28Z

Hey Tamir!

First of all, thank you for reviewing the code!

Regarding linting:
When running bun lint, it modifies the entire codebase, resulting in 100+ changes.
Is there a way to apply the linter to a specific file? Running bun lint <file path> also seems to modify everything.

Secondly, in CONTRIBUTING.md, there is a mention of the command bun check but I couldn’t get it to work. It says there is no script called "check."
Could you please clarify how to run this command correctly? - I believe this is the reason the code didn't pass the automation.

Regarding the visual aspects - any preferences you have will be applied

Of course, all your comments will be addressed, fixed, and modified accordingly.

Thanks again for the review! changes are on the way :)

tmr232 · 2025-03-20T20:22:59Z

Regarding linting: When running bun lint, it modifies the entire codebase, resulting in 100+ changes. Is there a way to apply the linter to a specific file? Running bun lint <file path> also seems to modify everything.

The linters passing for all files are required for merging the code, you can see the failing checks at https://github.com/tmr232/function-graph-overview/actions/runs/13949594615/job/39061318568?pr=134
If you're getting a large number of changes, especially in files you did not edit yourself, it is likely that your tools are not up to date as I recently replaced prettier with biome. Try running bun install again and then bun lint.

Secondly, in CONTRIBUTING.md, there is a mention of the command bun check but I couldn’t get it to work. It says there is no script called "check." Could you please clarify how to run this command correctly? - I believe this is the reason the code didn't pass the automation.

That's my bad - I changed the command from bun check to bun lint and forgot to update the docs.

… functions pure.

extracted the functions and changed app.svelte still working on fetching once in render func.

…nName function

…or debugging

…pport for additional node types checked with github urls

# Conflicts: # src/control-flow/function-utils.ts

Started testing function name extraction on C++ code. Found multiple issues that need fixing -this part seems to be the hardest so far. BUT hey, we're passing linter... :)

…tree-sitter site

tmr232 · 2025-08-22T14:53:37Z

+ * @param tag - The capture tag name to filter by.
+ *
+ */
+export function extractTaggedValueFromTreeSitterQuery(


This functionality is replicating the functionality of the block-matcher.ts file.
Unless you have a good reason not to use it - I'd prefer to used the existing code to maintain consistency.

Hi, so I'm trying to replace this function and I wanted to ask a few questions.

In the file block-matcher.ts, the function matchQuery returns only the first match.
That works pretty well for me, except for a couple of small cases.

In my function I'm using captures, which give me all the captures from all matches.
This is sometimes useful, for example when there are multiple assignments to multiple functions.
For instance, in Go:

var x, y = func() {}, func() {}

Maybe I should add a capture function to block-matcher.ts and export it?
I'd really like to hear a suggestion.

Just shooting an idea:
what if matchQuery would return all the matches,
but in the class the function match still return just the first one?
Then I could also add a function called matchAll that returns all the matches.

I did this in another branch earlier, it helps me a bit.
Do you want me to keep it, or better not change this file?
Link to diffs (only block-matcher.ts)

tmr232 · 2025-08-22T15:01:34Z

+  test("arrow variations", () => {
+    const code1 = "<T,>(value: T): T[] => {};";
+    expect(namesFrom(code1)).toEqual(["<anonymous>"]);
+
+    const code2 = `
+      function f() {
+        const b = n => n + 1;
+        const c = async () => { return 1; };
+        const d = () => () => {};
+      }
+    `;
+    expect(namesFrom(code2)).toEqual(["f", "b", "c", "d", "<anonymous>"]);


Please split this into 2 tests.

tmr232 · 2025-08-22T15:02:16Z

+  test("function expression variants", () => {
+    const code1 = "const myFunction = function(name: string): string {};";
+    expect(namesFrom(code1)).toEqual(["myFunction"]);
+
+    const code2 = "const sum = function add(): number {};";
+    expect(namesFrom(code2)).toEqual(["add"]);
+
+    const code3 = "function(name: string): string {};";
+    expect(namesFrom(code3)).toEqual(["<anonymous>"]);
+  });


These checks repeat the same pattern. Please make them into a parameterized test instead.
Repeats.

No problem, will do

…changes need to be done.

need to refactor tests and remove dups in all tests.

…g fully - need to refactor all the tests - need to refactor query-utils.ts to use block-matcher.ts

…tracted.

…ph-overview into issue#94 # Conflicts: # src/test/extractNamePython.test.ts

tmr232

Looks good!
Still a few changes needed, but they are minor.

tmr232 · 2025-09-05T19:30:55Z

+describe("Go: keyed elements - not supported", () => {
+  test("simple keys", () => {
+    const code = `
+      var _ = map[interface{}]func(){
+        "simple":        func() {},  
+        'r':             func() {},  
+        fmt.Sprint(1):   func() {},         
+        T(1):            func() {},        
+        [2]int{1,2}:     func() {},   
+        1 + 2:           func() {},     
+      }
+    `;
+    expect(namesFrom(code)).toEqual([
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+    ]);
+  });
+});


I'm sorry, but this doesn't really make sense to me. Feels really far fetched.

So should we remove it?

tmr232 · 2025-09-05T19:33:47Z

+/**
+ * Extracts the text content of syntax tree nodes captured by a Tree-sitter query.
+ *
+ * @param func - The syntax node from which to extract the tree.


Should be node, not func.

tmr232 · 2025-09-05T19:35:20Z

+ *
+ * @param func - The syntax node from which to extract the tree.
+ * @param query - The Tree-sitter query string to execute.
+ * @param tag - The capture tag name to filter by.


Rename to captureName to be consistent with tree-sitter terminology.

tmr232 · 2025-09-05T19:44:14Z

+ * Mapping of languages to their function name extraction functions.
+ * This ensures all supported languages have extractors via TypeScript typing.
+ */
+const functionNameExtractors: Record<


Let's move these into the language definition in each cfg-<language>.ts file, and then grab it from the mapping in cfg.ts instead of creating an extra one.

tmr232 · 2025-09-05T20:15:55Z

@@ -106,8 +157,10 @@ async function getFunctionByLine(
 function setBackgroundColor(colors: "light" | "dark") {


Probably better to rename to applyColors or similar, now that we're not only setting the background color!

tmr232 · 2025-09-05T20:18:27Z

+
    if (params.type === "GitHub") {
      codeUrl = params.codeUrl;
+      const { func, language } = await fetchFunctionAndLanguage(params);


I don't like that we're fetchign this twice, but I guess we'll have to do for now...

tmr232 · 2025-09-05T20:19:07Z

+  {#if functionAndCFGMetadata}
+   <!-- Metadata display -->
+{#if metadataFields.some(field => showMetadata[field.key] && field.value() !== undefined)}
+<div class="metadata" class:panel-open={isPanelOpen}>
+    {#each metadataFields.filter(field => field.value() !== undefined) as { key, label , value}}
+    {#if showMetadata[key]}
+      <span>{label}: {value()}</span>
+    {/if}
+  {/each}
+</div>
+{/if}
+    <button class="panel-toggle" onclick={togglePanel}>
+      {isPanelOpen ? '→' : '←'}
+    </button>
+    <!-- Control panel -->
+    <div class="control-panel" class:open={isPanelOpen}>
+      <h3>Display Options</h3>
+        {#each metadataFields.filter(field => field.value() !== undefined) as { key, label }}
+      <label>
+          <input type="checkbox" bind:checked={showMetadata[key]} />
+          {label}
+        </label>
+      {/each}
+    </div>
+  {/if}


Please fix the indentation here.

tmr232 · 2025-09-05T20:20:42Z

+  // Update CFG metadata
+  const nodeCount: number = CFG.graph.order;
+  const edgeCount: number = CFG.graph.size;
+  const cyclomaticComplexity: number = CFG.graph.size - nodeCount + 2; // (https://en.wikipedia.org/wiki/Cyclomatic_complexity)


Move the comment to above the line, no need for the line to be this long...

tmr232 · 2025-09-05T20:21:23Z

+  const cyclomaticComplexity: number = CFG.graph.size - nodeCount + 2; // (https://en.wikipedia.org/wiki/Cyclomatic_complexity)
+
+  return {
+    functionData: name && lineCount ? { name, lineCount, language } : undefined,


Please make this a proper if, or at least use parenthesis around the condition. Readability counts!

and cpp

removed an unused case in findVariableBinding for TypeScript

Hopefully I did it right, and buherator doesn't count as a surname :-)

tmr232

Good job!

AlonBilman · 2025-09-08T17:07:48Z

Thank you Tamir!
It was a great journey.
We learned a lot, and it means a lot that we made it happen :)

AlonBilman added 4 commits March 18, 2025 13:02

Added new metadata type, extracted data from cfg and the function.

f7b084e

extracting all the data for issue tmr232#94 , tested the extraction w…

3151ea6

…ith github repos

added the metadata to the page

ca084e1

added css

Updated .md files (changelog and readme)

7a7b13f

tmr232 changed the title ~~Issue#94~~ Adding function information to the /render page Mar 19, 2025

tmr232 self-requested a review March 19, 2025 18:36

tmr232 requested changes Mar 19, 2025

View reviewed changes

linting- commit in order to merge

2ae4d60

AlonBilman closed this Mar 23, 2025

AlonBilman deleted the issue#94 branch March 23, 2025 13:50

AlonBilman restored the issue#94 branch March 23, 2025 14:04

tmr232 reopened this Mar 23, 2025

Bennahmias and others added 16 commits March 26, 2025 14:22

check and update lint

d5bbffb

Trying to find a way to avoid fetching twice and to make the relevant…

a2ba7d5

… functions pure.

Merge remote-tracking branch 'origin/main' into issue#94

7618ad8

Added function name extraction in function-utils.ts

d675eac

extracted the functions and changed app.svelte still working on fetching once in render func.

Add: functionDetils.test.ts file that test for now the extractFunctio…

6e6d675

…nName function

Merge remote-tracking branch 'origin/issue#94' into issue#94_testing

e944b01

Fix: update import path for extractFunctionName and add console log f…

a71484b

…or debugging

Refactor function name extraction to improve type handling and add su…

34e3ae6

…pport for additional node types checked with github urls

Fixed a bug in the TypeScript generator's function name extraction.

328d86d

added documentation

dcea710

Refactor: update tests for extractFunctionName

69bf1a4

Merge remote-tracking branch 'origin/issue#94' into issue#94_testing

0d3f03e

# Conflicts: # src/control-flow/function-utils.ts

simplify anonymous function naming

c279feb

change the test suites

fac3bef

Merge remote-tracking branch 'origin/issue#94' into issue#94_testing

ce19dd5

fix: add some test cases

4d7f20f

AlonBilman and others added 4 commits August 20, 2025 13:16

Oops forgot to lint..

e461ea4

delete similar tests and open the cpp test file

f372e4c

test(cpp): add function name extraction tests and uncover major bugs

16eb3f1

Started testing function name extraction on C++ code. Found multiple issues that need fixing -this part seems to be the hardest so far. BUT hey, we're passing linter... :)

refactor(cpp): finished non-arrow function names after long fight in …

5da022e

…tree-sitter site

tmr232 reviewed Aug 22, 2025

View reviewed changes

AlonBilman added 13 commits August 25, 2025 21:15

refactor(c++): update C++ function name extraction and tests - small …

071fa8c

…changes need to be done.

cpp: finished extraction name logic.

a409c22

need to refactor tests and remove dups in all tests.

delete redundant Go and C++ function extraction tests

cd10232

refactor small things regarding extractions , lint + tests are passin…

be1f075

…g fully - need to refactor all the tests - need to refactor query-utils.ts to use block-matcher.ts

change example

96f7cf9

Refactor app.svelte to display "<anonymous>" when no name has been ex…

cf9a808

…tracted.

Update tests : replace "<anonymous>" with undefined

488d200

Refactor tests in extractNameTypescript.test.ts

40cef6d

Refactor Python tests

b731404

Refactor C ;Python ;Typescript tests

c05ddf2

Merge branch 'issue#94' of https://github.com/AlonBilman/function-gra…

734d70d

…ph-overview into issue#94 # Conflicts: # src/test/extractNamePython.test.ts

Refactor Go tests

e0603a7

Refactor C++ test cases

a586e5d

AlonBilman requested a review from tmr232 September 1, 2025 22:25

tmr232 reviewed Sep 5, 2025

View reviewed changes

Bennahmias and others added 5 commits September 7, 2025 20:32

fix PR changes

a5ca535

removed a test case for Go

0aebc93

Refactor findVariableBinding in go

baab71d

and cpp

add comments to binding functions

555194f

removed an unused case in findVariableBinding for TypeScript

Update CONTRIBUTORS.md

2330f49

Hopefully I did it right, and buherator doesn't count as a surname :-)

AlonBilman requested a review from tmr232 September 7, 2025 22:13

tmr232 approved these changes Sep 8, 2025

View reviewed changes

tmr232 merged commit de661a6 into tmr232:main Sep 8, 2025

tmr232 mentioned this pull request Sep 13, 2025

Add function info and stats to the /render page #94

Closed

		@@ -106,8 +157,10 @@ async function getFunctionByLine(
		function setBackgroundColor(colors: "light" \| "dark") {

Conversation

AlonBilman commented Mar 19, 2025

Uh oh!

tmr232 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlonBilman commented Mar 20, 2025

Uh oh!

tmr232 commented Mar 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tmr232 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmr232 left a comment

Choose a reason for hiding this comment

Uh oh!

AlonBilman commented Sep 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tmr232 left a comment •

edited

Loading