Skip to content
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

Implement Trace.to and Trace.toError() #1

Closed
janodvarko opened this issue Jun 12, 2014 · 16 comments
Closed

Implement Trace.to and Trace.toError() #1

janodvarko opened this issue Jun 12, 2014 · 16 comments
Assignees
Labels

Comments

@janodvarko
Copy link
Member

We need support for conditional tracing in Firebug.next

Use case:

var TraceError = Trace.toError();
var Trace = Trace.to("DBG_MYOPTION");

Honza

@janodvarko
Copy link
Member Author

@fflorent: Florent, I know you were already involved in this, perhaps you are interested in this issue...?

Honza

@fflorent
Copy link
Member

I was involved about the Trace.toError(), indeed.

Sure, I'll do that.

Florent

@janodvarko
Copy link
Member Author

Great, you might want to read instructions about how to install/run Firebug.next:
https://github.com/firebug/firebug.next/blob/master/README.md#hacking-on-firebugnext
(let me know if the instructions can be improved, so it's easier to follow them for the first time)

Honza

@janodvarko
Copy link
Member Author

One small and related commit:
3a9a61e
(just to have TraceError in place, so the rest of the code can start using it)

Honza

@fflorent
Copy link
Member

I am working on it.

Florent

fflorent added a commit to fflorent/firebug.next that referenced this issue Jul 12, 2014
fflorent added a commit to fflorent/firebug.next that referenced this issue Jul 13, 2014
fflorent added a commit to fflorent/firebug.next that referenced this issue Jul 13, 2014
@fflorent
Copy link
Member

That should work.
I had to make some changes in FBTrace code (see fflorent/firebug@b4ebb84).

I also changed some files to include const Trace = FBTrace.to("DBG_...");, but I don't know what DBG_... options we should use for each file. I can do the change for each of them if you tell me what you want.

Note for me (or any vimist who want to do the above changes for each file): here is the macro to automatically rename Trace to FBTrace and place const Trace = FBTrace.to("DBG_...");:

/Trace^MiFB^[GG?require(^Mo^Mconst Trace = FBTrace.to("DBG_...");^[

Florent

@janodvarko
Copy link
Member Author

Nice, I tested it and works for me, thanks!

but I don't know what DBG_... options we should use for each file. I can do the change
for each of them if you tell me what you want.

We have been usually using (capitalized) file name in the past, but think we should automatize this by using module.id (module global is automatically available in each module).

const Trace = FBTrace.to(module.id);

We can also introduce simplified syntax. What about:

const { Trace, TraceError } = require("./core/trace.js").to(module.id);

TraceError is not currently attached into the current file and not
returned by to(), so perhaps we can have additional get() API:

const { Trace, TraceError } = require("./core/trace.js").get(module.id);

The result pref name can be e.g:
extensions.firebug.firebug-next/chrome/chrome
... which should be syntactically ok, but I don't see it in the Options tab in the Trace console.

Since the option names tend to be rather longer (module IDs are paths), we might also want to display the list of available options (in the Options tab) as one column sorted alphabetically. It should be a lot faster to find the right option then.

Honza

@fflorent
Copy link
Member

We have been usually using (capitalized) file name in the past, but think we should automatize this by using module.id (module global is automatically available in each module).
const Trace = FBTrace.to(module.id);

Then the option panel should allow to select a folder (that will be definitely practical).

Also I propose this:

  1. In this issue
    1.1. Fix the problems you noticed
    1.2. Introduce FBTrace.get("module.id");, which returns the appropriate Trace and TraceError objects
    1.3. Map the module id to some DBG_ option
  2. In another issue
    2.1. Split the FBTrace project from the Firebug repository, so we are free to adapt it for the firebug.next needs
    2.2. Adapt the Option tab and remove the mapping between the module id and the DBG_... option

What do you think?

extensions.firebug.firebug-next/chrome/chrome

Then maybe rather extensions.firebug.fbtrace.firebug-next/chrome/chrome?

Florent

@janodvarko
Copy link
Member Author

Then the option panel should allow to select a folder (that will be definitely practical).

Yep, I like the idea!

  1. In this issue + In another issue

All sounds great to me.

Just one question, not sure what you mean by: Map the module id to some DBG_ option
Do you mean generate temporary DBG_* name from the module path?

Then maybe rather extensions.firebug.fbtrace.firebug-next/chrome/chrome?

Note that firebug-next become firebug 3 at some point and "firebug" is there twice already.
What about simplify a bit:

extensions.firebug.fbtrace.chrome/chrome

Honza

@fflorent
Copy link
Member

Do you mean generate temporary DBG_* name from the module path?

Exactly. Thus if module.id.contains("lib/console/") for instance, then the option would be DBG_CONSOLE, etc.

extensions.firebug.fbtrace.chrome/chrome

Sounds good to me.

Florent

@janodvarko
Copy link
Member Author

Exactly. Thus if module.id.contains("lib/console/") for instance, then the option would be
DBG_CONSOLE, etc.

I see, ok

Honza

@fflorent
Copy link
Member

@honza what do you think of the two above commits?
I introduced:

  • FBTrace.get(module.id) as we agreed, that will benefit from the improvements of the option tab
  • lib/core/traceModuleOptionMapping.js, which is where the mapping is configured (will be deleted as soon as the option tab is improved). If the mapping for a module id doesn't exist in this file, Trace logs everything into DBG_OTHERS.

If that's fine for you, I'll adapt the other files (I can automatize using shell scripts + vim macros), merge into master and I'll let you edit lib/core/traceModuleOptionMapping.js at your convenience.

Florent

@janodvarko
Copy link
Member Author

@honza what do you think of the two above commits?

I commented in the commit

Btw. you need to use @janodvarko ;-)

Honza

@fflorent
Copy link
Member

Right. Above the commits with your suggestions (+ firebug/firebug@eb3e7e2).

Florent

@janodvarko
Copy link
Member Author

Nice, works for me just fine!

I think you can merge (both modified branches) and close this issue

Related commits in the firebug/firebug repo:
firebug/firebug@eb3e7e2
firebug/firebug@b4ebb84
firebug/firebug@0ced84a

I created two follow ups as discussed above:

Honza

@fflorent
Copy link
Member

Merged in af94c9b

Florent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants