-
Notifications
You must be signed in to change notification settings - Fork 126
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
collects imports, removes bang line, and prepends at top of script #75
Changes from all commits
5c2128d
4dcd616
1c5f638
67eff8e
5ec6ed0
06b5415
e2d1794
f7d8e02
caa56e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,21 +9,21 @@ import java.net.URL | |
*/ | ||
|
||
/** Resolve include declarations in a script file. Resolved script will be put into another temporary script */ | ||
fun resolveIncludes(template: File?): File? { | ||
if (template == null) return null | ||
fun resolveIncludes(template: File): File = resolveIncludesInternal(template) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By doing so you removed the recursiveness. Before included files could contain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think its a YAGNI thing. Do you want it back? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally think it also better to leave only at the top level and not allow recursion as it avoids the possibility of circular includes. When we add the feature to allow a "preamble" file for the DSL functionality then all includes would only go in there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you could list includes in the preamble. And you're right about recursion YAGNIness. Not sure why I was obsessed enough in the first place to implement it that way. |
||
|
||
// recursively replace //INCLUDES | ||
return resolveIncludesInternal(template)?.run { resolveIncludes(this) } ?: template | ||
} | ||
|
||
private fun resolveIncludesInternal(template: File): File? { | ||
private fun resolveIncludesInternal(template: File): File { | ||
val IMPORT_TEXT = "import " | ||
val scriptLines = template.readText().lines() | ||
|
||
// don't do a thing if there's not INCLUDE in the script | ||
if (scriptLines.find { it.startsWith("//INCLUDE ") } == null) return null | ||
if (scriptLines.find { it.startsWith("//INCLUDE ") } == null) { | ||
return template | ||
} | ||
|
||
val sb = StringBuilder() | ||
|
||
// collect up the set of imports in this | ||
val imports : MutableSet<String> = emptySet<String>().toMutableSet() | ||
|
||
scriptLines.map { | ||
if (it.startsWith("//INCLUDE")) { | ||
val include = it.split("[ ]+".toRegex()).last() | ||
|
@@ -37,20 +37,32 @@ private fun resolveIncludesInternal(template: File): File? { | |
} | ||
|
||
try { | ||
sb.appendln(includeURL.readText()) | ||
// collect the import or emit | ||
includeURL.readText().lines().forEach { | ||
if (it.startsWith(IMPORT_TEXT)) { | ||
imports.add(it) | ||
} else { | ||
sb.appendln(it) | ||
} | ||
} | ||
} catch (e: FileNotFoundException) { | ||
errorMsg("Failed to resolve //INCLUDE '${include}'") | ||
System.err.println(e.message?.lines()!!.map { it.prependIndent("[ERROR] ") }) | ||
|
||
quit(1) | ||
} | ||
} else if (it.startsWith(IMPORT_TEXT)) { | ||
imports.add(it) | ||
} else { | ||
// if it's not a directive we simply skip emit the line as it is | ||
sb.appendln(it) | ||
// if its not an include directive or an import or a bang line, emit as is | ||
if (!it.startsWith("#!/")) { sb.appendln(it) } else { } | ||
} | ||
} | ||
|
||
return createTmpScript(sb.toString()) | ||
val impsb = StringBuilder() | ||
imports.map { impsb.appendln(it) } | ||
|
||
val final = impsb.appendln(sb.toString()) | ||
return createTmpScript(final.toString()) | ||
} | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import io.kotlintest.matchers.shouldBe | ||
import kscript.app.* | ||
import org.junit.Test | ||
import java.io.File | ||
import org.hamcrest.core.Is.* | ||
import org.junit.Assert.* | ||
|
||
class ConsolidateImports { | ||
|
||
@Test | ||
fun test_consolidate_imports() { | ||
val classLoader = javaClass.classLoader | ||
val file = this.javaClass.getResource("/consolidate_includes/input.script").file | ||
val expected = this.javaClass.getResource("/consolidate_includes/expected.script") | ||
val result = resolveIncludes(File(file)) | ||
|
||
assertThat(result.readText(),`is`(expected.readText())) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import here.we.go | ||
import we.go.here | ||
import go.here.we | ||
import if.not.now.when | ||
import if.not.me.then.who | ||
import my.cool.class | ||
//DEPS com.eclipsesource.minimal-json:minimal-json:0.9.4 | ||
|
||
file 1 | ||
|
||
Lastly, she pictured to herself how this same little sister of | ||
hers would, in the after-time, be herself a grown woman; and how | ||
she would keep, through all her riper years, the simple and | ||
loving heart of her childhood: and how she would gather about | ||
her other little children, and make THEIR eyes bright and eager | ||
with many a strange tale, perhaps even with the dream of | ||
Wonderland of long ago: and how she would feel with all their | ||
simple sorrows, and find a pleasure in all their simple joys, | ||
remembering her own child-life, and the happy summer days. | ||
|
||
|
||
file 2 | ||
|
||
Alice waited a little, half expecting to see it again, but it | ||
did not appear, and after a minute or two she walked on in the | ||
direction in which the March Hare was said to live. `I've seen | ||
hatters before,' she said to herself; `the March Hare will be | ||
much the most interesting, and perhaps as this is May it won't be | ||
raving mad--at least not so mad as it was in March.' As she said | ||
this, she looked up, and there was the Cat again, sitting on a | ||
branch of a tree. | ||
|
||
file 3 | ||
important to not treat me as import | ||
|
||
This time Alice waited patiently until it chose to speak again. | ||
In a minute or two the Caterpillar took the hookah out of its | ||
mouth and yawned once or twice, and shook itself. Then it got | ||
down off the mushroom, and crawled away in the grass, merely | ||
remarking as it went, `One side will make you grow taller, and | ||
the other side will make you grow shorter.' | ||
|
||
|
||
Script | ||
important to also not treat me as import | ||
|
||
Alice was beginning to get very tired of sitting by her sister | ||
on the bank, and of having nothing to do: once or twice she had | ||
peeped into the book her sister was reading, but it had no | ||
pictures or conversations in it, `and what is the use of a book,' | ||
thought Alice `without pictures or conversation?' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import here.we.go | ||
import we.go.here | ||
import go.here.we | ||
|
||
file 1 | ||
|
||
Lastly, she pictured to herself how this same little sister of | ||
hers would, in the after-time, be herself a grown woman; and how | ||
she would keep, through all her riper years, the simple and | ||
loving heart of her childhood: and how she would gather about | ||
her other little children, and make THEIR eyes bright and eager | ||
with many a strange tale, perhaps even with the dream of | ||
Wonderland of long ago: and how she would feel with all their | ||
simple sorrows, and find a pleasure in all their simple joys, | ||
remembering her own child-life, and the happy summer days. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import here.we.go | ||
import we.go.here | ||
import if.not.now.when | ||
|
||
file 2 | ||
|
||
Alice waited a little, half expecting to see it again, but it | ||
did not appear, and after a minute or two she walked on in the | ||
direction in which the March Hare was said to live. `I've seen | ||
hatters before,' she said to herself; `the March Hare will be | ||
much the most interesting, and perhaps as this is May it won't be | ||
raving mad--at least not so mad as it was in March.' As she said | ||
this, she looked up, and there was the Cat again, sitting on a | ||
branch of a tree. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import go.here.we | ||
import if.not.me.then.who | ||
|
||
file 3 | ||
important to not treat me as import | ||
|
||
This time Alice waited patiently until it chose to speak again. | ||
In a minute or two the Caterpillar took the hookah out of its | ||
mouth and yawned once or twice, and shook itself. Then it got | ||
down off the mushroom, and crawled away in the grass, merely | ||
remarking as it went, `One side will make you grow taller, and | ||
the other side will make you grow shorter.' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
#!/usr/bin/env kscript | ||
//DEPS com.eclipsesource.minimal-json:minimal-json:0.9.4 | ||
//INCLUDE file1 | ||
//INCLUDE file2 | ||
//INCLUDE file3 | ||
|
||
import my.cool.class | ||
|
||
Script | ||
important to also not treat me as import | ||
|
||
Alice was beginning to get very tired of sitting by her sister | ||
on the bank, and of having nothing to do: once or twice she had | ||
peeped into the book her sister was reading, but it had no | ||
pictures or conversations in it, `and what is the use of a book,' | ||
thought Alice `without pictures or conversation?' |
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.
the scriptfile can not be null, we should rather use !! or latinit here.
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.
You have a null check later in the code, so I thought maybe it could be null?
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.
file can be null as per "I do not exist" test that fails if I replace with !!, so this is necessary.