Issue 1130093007: Create html if needed in server mode

Issue 1130093007: Create html if needed in server mode (Closed)

Created:
5 years, 7 months ago by vsm

Modified:
5 years, 7 months ago

Reviewers:
Siggi Cherem (dart-lang), Jennifer Messerly

CC:
dev-compiler+reviews_dartlang.org

Base URL:
https://github.com/dart-lang/dev_compiler.git@master

Target Ref:
refs/heads/master

Visibility:
Public.

More Reviews

Description

Create html if needed in server mode This allows just serving up a dart file: dart ./bin/devc.dart --server foo.dart R=jmesserly@google.com Committed: https://github.com/dart-lang/dev_compiler/commit/ff0e176130405dda8e7bcb81331bd519383f1671

Patch Set 2 : Revert karma change #

Patch Set 3 : Address comments #

Created: 5 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+163 lines, -102 lines)			Patch
M	lib/devc.dart	View	1 2	4 chunks	+16 lines, -8 lines	0 comments	Download
M	lib/src/checker/resolver.dart	View	1 2	4 chunks	+22 lines, -3 lines	0 comments	Download
A	lib/src/in_memory.dart	View		1 chunk	+94 lines, -0 lines	0 comments	Download
M	lib/src/options.dart	View	1 2	5 chunks	+19 lines, -4 lines	0 comments	Download
M	lib/src/testing.dart	View		5 chunks	+5 lines, -80 lines	0 comments	Download
M	test/dependency_graph_test.dart	View		6 chunks	+7 lines, -7 lines	0 comments	Download

PTAL https://codereview.chromium.org/1130093007/diff/1/lib/devc.dart File lib/devc.dart (right): https://codereview.chromium.org/1130093007/diff/1/lib/devc.dart#newcode69 lib/devc.dart:69: InMemoryUriResolver entryResolver = null; On 2015/05/13 19:31:55, John ...

5 years, 7 months ago (2015-05-14 00:04:08 UTC) #5

PTAL

https://codereview.chromium.org/1130093007/diff/1/lib/devc.dart
File lib/devc.dart (right):

https://codereview.chromium.org/1130093007/diff/1/lib/devc.dart#newcode69
lib/devc.dart:69: InMemoryUriResolver entryResolver = null;
On 2015/05/13 19:31:55, John Messerly wrote:
> per suggestion below, moving implicitHtml into options, this code can move
into
> TypeResolver's constructor.
> 
> IMO, it makes more sense there, since that's where we are making the resolver
> list.
> 
> Alternatively, perhaps resolvers needs to be pulled out here. There was
already
> the `otherResolvers` hack ... adding entryResolver seems to make it even more
> crazy :)

Done.  Pushed it down to Resolver.

https://codereview.chromium.org/1130093007/diff/1/lib/devc.dart#newcode77
lib/devc.dart:77: '<html><body><script type="application/dart"
src="$src"></script></body></html>'
On 2015/05/13 19:31:55, John Messerly wrote:
> long line. sadly formatter won't fix these or even warn about them :|

Moved.

https://codereview.chromium.org/1130093007/diff/1/lib/devc.dart#newcode271
lib/devc.dart:271: bool implicitHtml = false;
On 2015/05/13 19:31:55, John Messerly wrote:
> suggestion: move this into options. It can still be a computed property:
> 
>     get implicitHtml => entryPointFile.endsWith('.dart');
> 
> or alternatively computed when we build up options. Either way, that would
mean
> less state to flow around, because both Compiler/CompilerServer have access to
> `options`

Done.

https://codereview.chromium.org/1130093007/diff/1/lib/src/in_memory.dart
File lib/src/in_memory.dart (right):

https://codereview.chromium.org/1130093007/diff/1/lib/src/in_memory.dart#newc...
lib/src/in_memory.dart:16: class InMemoryUriResolver extends UriResolver {
On 2015/05/13 19:31:55, John Messerly wrote:
> UPDATE: just saw this was from our tests (next file in review). I must've not
> noticed this code before. Well, still may be good to fix comments, so I left
> them :)
> 
> 
> general note ... as part of fixing server bug, I've been wanting to clean up
> duplication between us and analyzer. This seems to duplicate
> file_system/memory_file_system.dart, from analyzer, which can be used via `new
> ResourceUriResolver(new MemoryResourceProvider())`
> 
> analysis_server package uses it for stuff too, so it seems safe to depend on.

Yeah, this is just the old stuff with s/Test/InMemory/ otherwise unmodified.

I think you are right about MemoryResourceProvider.  I'm looking at that in a
separate CL.  Might be able to kill this file.

Jennifer Messerly

lgtm! https://codereview.chromium.org/1130093007/diff/1/lib/src/in_memory.dart File lib/src/in_memory.dart (right): https://codereview.chromium.org/1130093007/diff/1/lib/src/in_memory.dart#newcode16 lib/src/in_memory.dart:16: class InMemoryUriResolver extends UriResolver { On 2015/05/14 00:04:08, ...

5 years, 7 months ago (2015-05-14 14:43:16 UTC) #6

vsm

5 years, 7 months ago (2015-05-14 16:26:30 UTC) #7

Message was sent while issue was closed.

Committed patchset #3 (id:40001) manually as
ff0e176130405dda8e7bcb81331bd519383f1671 (presubmit successful).

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages