Copying a function doesn't set the line number

[faho]

Define func.fish with the following contents:

function t1
    echo t1
end

functions -c t1 t2

functions t1
functions t2

and run it as fish func.fish.

This prints:

# Defined in func.fish @ line 1
function t1
    echo t1
end
function t2
    echo t1
end

Notice how t2 does not print a line number? I feel it should, specifically line 5.

I noticed this while porting the "function.in" test to littlecheck, because that runs the tests as a file instead of via stdin.

[esdmr]

I was writing a script to setup my OS after reinstall when I noticed this too. I was doing something like:

for i in (functions -n | string match -re '^setup_')
    functions --copy $i _original_$i

    eval "function $i
    _task_begin || return
    _original_$i || return
    _task_end || return
end"
end

Inside of _original_$1, I call status stack-trace if there were any errors. But I noticed that the trace did not contain any line numbers.

After some git blames, I found the source to be a single line from 13 years ago, since the first time --copy (or rather --rename) was implemented back in v2.0.0.

Currently, the line has a comment specifying that: “This new instance of the function shouldn't be tied to the definition file of the […] original, so clear the filename, etc.” Other than the comment, I did not find any rationales for it.

I think that it was because the original definition did not have the same name and that was deemed confusing.

I tried removing that line:

-new_props->definition_file = nullptr;

And it seems to work (?).

function hello
    status stack-trace
end

function world
    hello
end

echo ── Normal ──────────────────────────────────────────────────

world
functions hello
functions world

echo ── Copied ──────────────────────────────────────────────────

functions -c world world2

world2
functions world2

Normal fish output

── Normal ──────────────────────────────────────────────────
in function 'hello'
	called on line 6 of file test.fish
in function 'world'
	called on line 11 of file test.fish
# Defined in test.fish @ line 1
function hello
    status stack-trace
end
# Defined in test.fish @ line 5
function world
    hello
end
── Copied ──────────────────────────────────────────────────
in function 'hello'
in function 'world2'
	called on line 19 of file test.fish
# Defined interactively
function world2
    hello
end

In second case, hello does not get a line number. Additionally, fish considers world2 to be “Defined interactively.”

Modified fish output

── Normal ──────────────────────────────────────────────────
in function 'hello'
	called on line 6 of file test.fish
in function 'world'
	called on line 11 of file test.fish
# Defined in test.fish @ line 1
function hello
    status stack-trace
end
# Defined in test.fish @ line 5
function world
    hello
end
── Copied ──────────────────────────────────────────────────
in function 'hello'
	called on line 6 of file test.fish
in function 'world2'
	called on line 19 of file test.fish
# Defined in test.fish @ line 5
function world2
    hello
end

In second case, hello gets the same line number as the first case. Fish considers world2 to be defined in the same place as world. (Would this be confusing for new users?)

[faho]

In second case, hello gets the same line number as the first case. Fish considers world2 to be defined in the same place as world. (Would this be confusing for new users?)

That is, in fact, why those lines exist.

What it probably should be doing instead is replace the definition line with where the copy happens?

[esdmr]

What it probably should be doing instead is replace the definition line with where the copy happens?

That makes sense for the function definition itself (# Defined in func.fish @ line 1).

But what about the lines in stack trace? Currently, it does not print those. I suggest we keep them, since copying does not modify the function.

(Alternatively, it could say it like: Defined in func.fish @ line 1, Renamed at func.fish @ line 5. Although, I do not know the preformance impact of including that information for every single function defined ever.)

(PS. Is stack trace in-scope for this issue, or should I make a new one?)

[faho]

(PS. Is stack trace in-scope for this issue, or should I make a new one?)

If the error is that stack-trace doesn't find the info, then that is very much the same thing.

Alternatively it could say it like: Defined in func.fish @ line 1, Renamed at func.fish @ line 5

That would probably be the right approach, yes.

I do not know the preformance impact of including that information for every single function defined ever

It's a single location per function definition. I do not believe this would be a big issue.

[esdmr]

Then I will see if I can send a PR. (I am not a C++ dev, so this might take some time.)