-
Notifications
You must be signed in to change notification settings - Fork 583
Add emitter scaffolding #172
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
Conversation
The program parts look pretty good to me. The only thing that gave me pause was whether or not |
I've moved it to program.go. I kept it separate mostly to avoid any potentially messy merge conflicts as I'd started this before #107 had merged. |
if err := vfs.writeFile(path, content, writeByteOrderMark); err == nil { | ||
return nil | ||
} | ||
if err := vfs.ensureDirectoryExists(tspath.GetDirectoryPath(tspath.NormalizePath(path))); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get away with not normalizing again? I think all callers are normalized.
(I would really like to explore using a named NormalizedPath
or something to avoid extra work...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no guarantee the path is normalized, just as there's no guarantee the path is rooted (which is why this and other methods validate the path is rooted). We also call NormalizePath
for other VFS methods, though it's buried in splitPath
/rootAndPath
, so it makes sense to remain consistent.
} | ||
|
||
// NOTE: EmitHost operations must be thread-safe | ||
type EmitHost interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we ever going to have more than one implementation of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% certain yet. This was something we allowed users to override when consuming the API, so that all depends on our long term API story (as in, could someone build on top of our go package). I opted to leave it as-is to more faithfully port portions of the emitter.
I think this looks good, but once last thing to check; since our program construction is more or less fake right now, where we just walk a dir and find all ts/tsx files, will this end up emitting files we shouldn't? I assume so, which can be fine, but we're going to start loading |
It won't generate emit for .d.ts input files at present, and only emits if you specify --outdir, so the risk is fairly low. |
Yeah, that seems fine, then. I was just thinking of random |
cmd/tsgo/main.go
Outdated
compilerOptions := &core.CompilerOptions{Strict: core.TSTrue, Target: core.ScriptTargetESNext, ModuleKind: core.ModuleKindNodeNext, NoEmit: core.TSTrue} | ||
if len(outDir) > 0 { | ||
compilerOptions.NoEmit = core.TSFalse | ||
compilerOptions.OutDir = outDir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just ran this; this needs to be something like:
compilerOptions.OutDir = outDir | |
compilerOptions.OutDir = tspath.ResolvePath(currentDirectory, outDir) |
Otherwise I get:
panic: vfs: path "wow/src/vs/platform/utilityProcess/common/utilityProcessWorkerService.js" is not absolute
(so, probably move down below)
Interestingly since we currently include
|
Excluding Node_modules is one of the as-yet unimplemented branches |
This adds
Program.Emit()
and the basic scaffolding for the emitter. At present, this only performs printback of the input source file as output transformation has not yet been implemented.This also adds an
--outDir DIR
option totsgo.exe
to verify emit. When not specified,tsgo.exe
will not trigger emit.In addition, this fixes a bug in
getLinesBetweenPositions
that incorrectly caused the printer to print excess newlines.