Bug 90529 – Web Inspector: Add FileContentView for FileSystemView

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Taiju Tsuiki]

For FileSystem support of Inspector, I'd like to add a view to show content of a file.
This patch adds FileContentView, this view is only for text file.

[Taiju Tsuiki]

Created attachment 150738 [details]
Patch

[Vsevolod Vlasov]

Comment on attachment 150738 [details]
Patch

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

> Source/WebCore/inspector/front-end/FileContentView.js:49
> +            if (this._file.resourceType.isTextType())

You should use another way to detect text files, see https://bugs.webkit.org/show_bug.cgi?id=90518 for details.

> Source/WebCore/inspector/front-end/FileContentView.js:61
> +        if (errorCode !== 0 || !metadata)

if (errorCode || !metadata)

> Source/WebCore/inspector/front-end/FileContentView.js:82
> +        if (this._file.resourceType.isTextType())

Ditto.

> Source/WebCore/inspector/front-end/FileContentView.js:95
> +WebInspector.FileContentView.Content = function(file, metadata)

WebInspector.FileContentProvider
I would move all this logic to WebInspector.FileSystemModel.File actually.

> Source/WebCore/inspector/front-end/FileSystemView.js:114
> +            if (this._entry.isDirectory) {

Please remove braces.

> Source/WebCore/inspector/front-end/FileSystemView.js:196
> +        } else {

No braces for one liners.

[Taiju Tsuiki]

Created attachment 151217 [details]
Patch

[Taiju Tsuiki]

Comment on attachment 150738 [details]
Patch

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

>> Source/WebCore/inspector/front-end/FileContentView.js:49
>> +            if (this._file.resourceType.isTextType())
> 
> You should use another way to detect text files, see https://bugs.webkit.org/show_bug.cgi?id=90518 for details.

I moved it to back-end and added isTextFile property.

>> Source/WebCore/inspector/front-end/FileContentView.js:61
>> +        if (errorCode !== 0 || !metadata)
> 
> if (errorCode || !metadata)

Done

>> Source/WebCore/inspector/front-end/FileContentView.js:95
>> +WebInspector.FileContentView.Content = function(file, metadata)
> 
> WebInspector.FileContentProvider
> I would move all this logic to WebInspector.FileSystemModel.File actually.

Renamed to FileContentProvider.
I'd like to separate file and metadata to keep file immutable.
Anyway, should I move FileContentProvider under FileSystemModel?

>> Source/WebCore/inspector/front-end/FileSystemView.js:114
>> +            if (this._entry.isDirectory) {
> 
> Please remove braces.

Done

>> Source/WebCore/inspector/front-end/FileSystemView.js:196
>> +        } else {
> 
> No braces for one liners.

Done

[Vsevolod Vlasov]

> Anyway, should I move FileContentProvider under FileSystemModel?
Sounds reasonable.

[WebKit Review Bot]

Comment on attachment 151217 [details]
Patch

Clearing flags on attachment: 151217

Committed r122104: <http://trac.webkit.org/changeset/122104>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.