Bug 54670 - Web Inspector: merge ConsoleView into ConsolePanel.
Summary: Web Inspector: merge ConsoleView into ConsolePanel.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Podivilov
URL:
Keywords:
Depends on: 60958
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-17 09:47 PST by Pavel Podivilov
Modified: 2011-05-17 07:33 PDT (History)
10 users (show)

See Also:


Attachments
Patch. (100.07 KB, patch)
2011-02-17 09:49 PST, Pavel Podivilov
pfeldman: review-
Details | Formatted Diff | Diff
Diff between old ConsoleView and new ConsolePanel (4.22 KB, patch)
2011-02-17 09:50 PST, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch. (10.26 KB, patch)
2011-04-29 05:09 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch. (11.57 KB, patch)
2011-05-04 07:25 PDT, Pavel Podivilov
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2011-02-17 09:47:19 PST
Web Inspector: merge ConsoleView into ConsolePanel.

ConsolePanel is ConsoleView shown in a fully opened drawer. We can easily merge this classes together so ConsoleView can reuse Panel functionality.
This is needed to implement snippet editor as sidebar pane in console panel.
Comment 1 Pavel Podivilov 2011-02-17 09:49:23 PST
Created attachment 82824 [details]
Patch.

- Add ConsoleView functions to ConsolePanel
- ConsoleView class
- Rename ConsoleView.js to ConsoleMessage.js
Comment 2 Pavel Podivilov 2011-02-17 09:50:59 PST
Created attachment 82825 [details]
Diff between old ConsoleView and new ConsolePanel
Comment 3 Pavel Feldman 2011-02-17 09:52:45 PST
Comment on attachment 82824 [details]
Patch.

Please provide diff with 
git config
[diff]
        renames = copies
Comment 4 Pavel Podivilov 2011-04-29 05:09:40 PDT
Created attachment 91666 [details]
Patch.

No moves/renames, just make ConsoleView a panel.
Comment 5 Yury Semikhatsky 2011-05-04 04:57:08 PDT
Comment on attachment 91666 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=91666&action=review

> Source/WebCore/ChangeLog:7
> +

Please give a brief description of the changes made. It's not clear in particular how conversion of ConsoleView into a panel helps us to add implement snippet editor.

> Source/WebCore/inspector/front-end/ConsoleView.js:32
> +WebInspector.ConsolePanel = function(drawer)

Containing file should be renamed as well.

> Source/WebCore/inspector/front-end/ConsoleView.js:248
> +        this._previousConsoleState = WebInspector.drawer.state;

This code should be removed from ConsolePanel.js, shouldn't it?

> Source/WebCore/inspector/front-end/inspector.js:192
> +            this.panels.console = this.console;

This method is supposed to create panels.
Comment 6 Pavel Podivilov 2011-05-04 07:25:58 PDT
Created attachment 92238 [details]
Patch.

Console view in drawer looks exactly the same as console panel. Merging ConsoleView and ConsolePanel together
will allow us to reuse panel's functionality (e.g. resizable sidebar) even when console is docked.
Comment 7 Pavel Podivilov 2011-05-04 07:27:04 PDT
(In reply to comment #5)
> (From update of attachment 91666 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91666&action=review
> 
> > Source/WebCore/ChangeLog:7
> > +
> 
> Please give a brief description of the changes made. It's not clear in particular how conversion of ConsoleView into a panel helps us to add implement snippet editor.

Done.

> 
> > Source/WebCore/inspector/front-end/ConsoleView.js:32
> > +WebInspector.ConsolePanel = function(drawer)
> 
> Containing file should be renamed as well.
> 
> > Source/WebCore/inspector/front-end/ConsoleView.js:248
> > +        this._previousConsoleState = WebInspector.drawer.state;
> 
> This code should be removed from ConsolePanel.js, shouldn't it?

Let me do it in a follow up patch.

> 
> > Source/WebCore/inspector/front-end/inspector.js:192
> > +            this.panels.console = this.console;
> 
> This method is supposed to create panels.

Done.
Comment 8 Pavel Podivilov 2011-05-16 10:54:55 PDT
Committed r86589: <http://trac.webkit.org/changeset/86589>
Comment 10 Andrew Wilson 2011-05-16 11:59:07 PDT
Reverted r86589 for reason:

Broke

Committed r86595: <http://trac.webkit.org/changeset/86595>
Comment 11 Pavel Podivilov 2011-05-17 02:01:19 PDT
Committed r86660: <http://trac.webkit.org/changeset/86660>