-
Notifications
You must be signed in to change notification settings - Fork 520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a combined source without #line directives into the dist package #363
Conversation
The file/line metadata map is useful for Duktape development: with a combined source traceback line it can resolved to an original source file/line, e.g.:
|
I'll give a few days before merging this so that if anyone has a strong opinion one way or another they'll have some time to comment. The concrete impacts of this change are:
To illustrate the traceback issue, before the change:
After the change:
The impact for troubleshooting is that if the Duktape source file/line matters, you'll need to figure out the uncombined file/line from the combined line somehow. The pull adds a utility to do that, and adds the necessary metadata into the dist, so that this will be relatively easy at least for official releases. |
By the way MSVC behaves similarly to gdb, which threw me off at first: it won't step into duktape.c but instead prompts for the location of the original source files. Normally I don't mind but it can become an issue when prototyping changes directly in the combined source as then the two copies don't match. On the other hand, it adds an extra level of indirection when deciphering debug prints. So there are pros and cons, and I'm not really sure which way is better. |
For application programming the impact in tracebacks should be mostly minimal: tracebacks have a function With lightfuncs it's a bit different because the virtual lightfunc |
Hm, debugging may actually be hindered by this: It seems MSVC doesn't like source files with more than 64k lines: |
That sounds vaguely familiar. So, without adding both combined source variants we'll have to choose between either:
We can add both file variants into the distributable if that's the only workable option though. It's just a bit awkward to explain. Concretely this would mean either adding a third source directory (so e.g. |
No consensus here yet? :) |
Any choice will lead to grumbling so I've yet to determine the path of least grumbling ;) I'll try to get this merged soonish, I've been busy with some other stuff. |
22a33b2
to
2b9b5cb
Compare
Ok, so as a compromise the pull now provides the following dist source structure:
The dist README has this note:
Hopefully this is a reasonable compromise :) |
2b9b5cb
to
830b8e1
Compare
This seems reasonable. I like that you chose to put it in a separate directory, keeps things neat. |
Yeah, I thought it best not to change the |
8160d08
to
ffcf499
Compare
…ives Add a combined source without #line directives into the dist package
Thanks, everyone. I'll be updating for the next release on Soletta soon. |
I'm a bit confused -- are there any releases contemplating this change yet? |
It's in master, which means it will be in the next Duktape release (1.4.0). |
Thanks a lot :) |
Related to #362. Prototype how duktape.c would work without #line directives:
--line-directives
option to combine_src.py