From f9a5fee3642936386fa47c63afe64900f3928563 Mon Sep 17 00:00:00 2001
From: Lucien Greathouse <me@lpghatguy.com>
Date: Tue, 1 Oct 2019 18:23:29 -0700
Subject: [PATCH] Reorganize and clean up plugin

---
 bin/run-plugin-tests.sh                       | 10 +---
 bin/run-tests.sh                              |  7 ---
 plugin/.luacheckrc                            |  1 +
 plugin/default.project.json                   |  6 +++
 plugin/{src/HttpError.lua => http/Error.lua}  | 34 +++++-------
 .../HttpResponse.lua => http/Response.lua}    | 18 +++----
 plugin/{src/Http.lua => http/init.lua}        | 24 +++++----
 plugin/http/init.spec.lua                     |  5 ++
 plugin/log/init.lua                           | 54 +++++++++++++++++++
 plugin/log/init.spec.lua                      |  5 ++
 plugin/src/ApiContext.lua                     |  5 +-
 plugin/src/Components/RojoFooter.lua          |  2 -
 plugin/src/Logging.lua                        | 52 +-----------------
 plugin/src/Reconciler.lua                     |  5 +-
 plugin/src/init.server.lua                    |  8 +++
 plugin/testBootstrap.server.lua               |  2 +-
 16 files changed, 125 insertions(+), 113 deletions(-)
 delete mode 100755 bin/run-tests.sh
 rename plugin/{src/HttpError.lua => http/Error.lua} (58%)
 rename plugin/{src/HttpResponse.lua => http/Response.lua} (56%)
 rename plugin/{src/Http.lua => http/init.lua} (63%)
 create mode 100644 plugin/http/init.spec.lua
 create mode 100644 plugin/log/init.lua
 create mode 100644 plugin/log/init.spec.lua

diff --git a/bin/run-plugin-tests.sh b/bin/run-plugin-tests.sh
index ecbf0c96..4f99bd10 100755
--- a/bin/run-plugin-tests.sh
+++ b/bin/run-plugin-tests.sh
@@ -14,13 +14,5 @@ remodel bin/put-plugin-in-test-place.lua "$PLUGIN_FILE" "$PLACE_FILE"
 run-in-roblox -s plugin/testBootstrap.server.lua "$PLACE_FILE"
 
 pushd plugin
-luacheck src
-popd
-
-pushd rojo-plugin-http
-luacheck src
-popd
-
-pushd rojo-plugin-log
-luacheck src
+luacheck src log http
 popd
\ No newline at end of file
diff --git a/bin/run-tests.sh b/bin/run-tests.sh
deleted file mode 100755
index 03647950..00000000
--- a/bin/run-tests.sh
+++ /dev/null
@@ -1,7 +0,0 @@
-#!/bin/sh
-
-set -ev
-
-cargo test --all --locked --verbose
-cargo fmt -- --check
-cargo clippy
\ No newline at end of file
diff --git a/plugin/.luacheckrc b/plugin/.luacheckrc
index 9a0c40a1..b523c1b5 100644
--- a/plugin/.luacheckrc
+++ b/plugin/.luacheckrc
@@ -20,6 +20,7 @@ stds.roblox = {
 		"CFrame",
 		"Enum",
 		"Instance",
+		"DockWidgetPluginGuiInfo",
 	}
 }
 
diff --git a/plugin/default.project.json b/plugin/default.project.json
index 791edc0e..bdccf678 100644
--- a/plugin/default.project.json
+++ b/plugin/default.project.json
@@ -5,6 +5,12 @@
     "Plugin": {
       "$path": "src"
     },
+    "Log": {
+      "$path": "log"
+    },
+    "Http": {
+      "$path": "http"
+    },
     "Roact": {
       "$path": "modules/roact/src"
     },
diff --git a/plugin/src/HttpError.lua b/plugin/http/Error.lua
similarity index 58%
rename from plugin/src/HttpError.lua
rename to plugin/http/Error.lua
index e1ef9b7a..43d6782b 100644
--- a/plugin/src/HttpError.lua
+++ b/plugin/http/Error.lua
@@ -1,9 +1,7 @@
-local Logging = require(script.Parent.Logging)
+local Error = {}
+Error.__index = Error
 
-local HttpError = {}
-HttpError.__index = HttpError
-
-HttpError.Error = {
+Error.Kind = {
 	HttpNotEnabled = {
 		message = "Rojo requires HTTP access, which is not enabled.\n" ..
 			"Check your game settings, located in the 'Home' tab of Studio.",
@@ -20,13 +18,13 @@ HttpError.Error = {
 	},
 }
 
-setmetatable(HttpError.Error, {
+setmetatable(Error.Kind, {
 	__index = function(_, key)
-		error(("%q is not a valid member of HttpError.Error"):format(tostring(key)), 2)
+		error(("%q is not a valid member of Http.Error.Kind"):format(tostring(key)), 2)
 	end,
 })
 
-function HttpError.new(type, extraMessage)
+function Error.new(type, extraMessage)
 	extraMessage = extraMessage or ""
 	local message = type.message:gsub("{{message}}", extraMessage)
 
@@ -35,38 +33,34 @@ function HttpError.new(type, extraMessage)
 		message = message,
 	}
 
-	setmetatable(err, HttpError)
+	setmetatable(err, Error)
 
 	return err
 end
 
-function HttpError:__tostring()
+function Error:__tostring()
 	return self.message
 end
 
 --[[
 	This method shouldn't have to exist. Ugh.
 ]]
-function HttpError.fromErrorString(message)
+function Error.fromErrorString(message)
 	local lower = message:lower()
 
 	if lower:find("^http requests are not enabled") then
-		return HttpError.new(HttpError.Error.HttpNotEnabled)
+		return Error.new(Error.Kind.HttpNotEnabled)
 	end
 
 	if lower:find("^httperror: timedout") then
-		return HttpError.new(HttpError.Error.Timeout)
+		return Error.new(Error.Kind.Timeout)
 	end
 
 	if lower:find("^httperror: connectfail") then
-		return HttpError.new(HttpError.Error.ConnectFailed)
+		return Error.new(Error.Kind.ConnectFailed)
 	end
 
-	return HttpError.new(HttpError.Error.Unknown, message)
+	return Error.new(Error.Kind.Unknown, message)
 end
 
-function HttpError:report()
-	Logging.warn(self.message)
-end
-
-return HttpError
\ No newline at end of file
+return Error
\ No newline at end of file
diff --git a/plugin/src/HttpResponse.lua b/plugin/http/Response.lua
similarity index 56%
rename from plugin/src/HttpResponse.lua
rename to plugin/http/Response.lua
index 039f8538..ce4e1b6d 100644
--- a/plugin/src/HttpResponse.lua
+++ b/plugin/http/Response.lua
@@ -1,34 +1,34 @@
 local HttpService = game:GetService("HttpService")
 
 local stringTemplate = [[
-HttpResponse {
+Http.Response {
 	code: %d
 	body: %s
 }]]
 
-local HttpResponse = {}
-HttpResponse.__index = HttpResponse
+local Response = {}
+Response.__index = Response
 
-function HttpResponse:__tostring()
+function Response:__tostring()
 	return stringTemplate:format(self.code, self.body)
 end
 
-function HttpResponse.fromRobloxResponse(response)
+function Response.fromRobloxResponse(response)
 	local self = {
 		body = response.Body,
 		code = response.StatusCode,
 		headers = response.Headers,
 	}
 
-	return setmetatable(self, HttpResponse)
+	return setmetatable(self, Response)
 end
 
-function HttpResponse:isSuccess()
+function Response:isSuccess()
 	return self.code >= 200 and self.code < 300
 end
 
-function HttpResponse:json()
+function Response:json()
 	return HttpService:JSONDecode(self.body)
 end
 
-return HttpResponse
\ No newline at end of file
+return Response
\ No newline at end of file
diff --git a/plugin/src/Http.lua b/plugin/http/init.lua
similarity index 63%
rename from plugin/src/Http.lua
rename to plugin/http/init.lua
index 03b26658..3cd016bf 100644
--- a/plugin/src/Http.lua
+++ b/plugin/http/init.lua
@@ -1,21 +1,23 @@
 local HttpService = game:GetService("HttpService")
 
-local Promise = require(script.Parent.Parent.Promise)
+local Promise = require(script.Parent.Promise)
+local Log = require(script.Parent.Log)
 
-local Logging = require(script.Parent.Logging)
-local HttpError = require(script.Parent.HttpError)
-local HttpResponse = require(script.Parent.HttpResponse)
+local HttpError = require(script.Error)
+local HttpResponse = require(script.Response)
 
 local lastRequestId = 0
 
--- TODO: Factor out into separate library, especially error handling
 local Http = {}
 
+Http.Error = HttpError
+Http.Response = HttpResponse
+
 function Http.get(url)
 	local requestId = lastRequestId + 1
 	lastRequestId = requestId
 
-	Logging.trace("GET(%d) %s", requestId, url)
+	Log.trace("GET(%d) %s", requestId, url)
 
 	return Promise.new(function(resolve, reject)
 		coroutine.wrap(function()
@@ -27,10 +29,10 @@ function Http.get(url)
 			end)
 
 			if success then
-				Logging.trace("Request %d success: status code %s", requestId, response.StatusCode)
+				Log.trace("Request %d success: status code %s", requestId, response.StatusCode)
 				resolve(HttpResponse.fromRobloxResponse(response))
 			else
-				Logging.trace("Request %d failure: %s", requestId, response)
+				Log.trace("Request %d failure: %s", requestId, response)
 				reject(HttpError.fromErrorString(response))
 			end
 		end)()
@@ -41,7 +43,7 @@ function Http.post(url, body)
 	local requestId = lastRequestId + 1
 	lastRequestId = requestId
 
-	Logging.trace("POST(%d) %s\n%s", requestId, url, body)
+	Log.trace("POST(%d) %s\n%s", requestId, url, body)
 
 	return Promise.new(function(resolve, reject)
 		coroutine.wrap(function()
@@ -54,10 +56,10 @@ function Http.post(url, body)
 			end)
 
 			if success then
-				Logging.trace("Request %d success: status code %s", requestId, response.StatusCode)
+				Log.trace("Request %d success: status code %s", requestId, response.StatusCode)
 				resolve(HttpResponse.fromRobloxResponse(response))
 			else
-				Logging.trace("Request %d failure: %s", requestId, response)
+				Log.trace("Request %d failure: %s", requestId, response)
 				reject(HttpError.fromErrorString(response))
 			end
 		end)()
diff --git a/plugin/http/init.spec.lua b/plugin/http/init.spec.lua
new file mode 100644
index 00000000..ff7e8e3b
--- /dev/null
+++ b/plugin/http/init.spec.lua
@@ -0,0 +1,5 @@
+return function()
+	it("should load", function()
+		require(script.Parent)
+	end)
+end
\ No newline at end of file
diff --git a/plugin/log/init.lua b/plugin/log/init.lua
new file mode 100644
index 00000000..d3defd6d
--- /dev/null
+++ b/plugin/log/init.lua
@@ -0,0 +1,54 @@
+local Level = {
+	Error = 0,
+	Warning = 1,
+	Info = 2,
+	Debug = 3,
+	Trace = 4,
+}
+
+local function getLogLevel()
+	return Level.Info
+end
+
+local function addTags(tag, message)
+	return tag .. message:gsub("\n", "\n" .. tag)
+end
+
+local TRACE_TAG = (" "):rep(15) .. "[Rojo-Trace] "
+local INFO_TAG = (" "):rep(15) .. "[Rojo-Info] "
+local DEBUG_TAG = (" "):rep(15) .. "[Rojo-Debug] "
+local WARN_TAG = "[Rojo-Warn] "
+
+local Log = {}
+
+Log.Level = Level
+
+function Log.setLogLevelThunk(thunk)
+	getLogLevel = thunk
+end
+
+function Log.trace(template, ...)
+	if getLogLevel() >= Level.Trace then
+		print(addTags(TRACE_TAG, string.format(template, ...)))
+	end
+end
+
+function Log.info(template, ...)
+	if getLogLevel() >= Level.Info then
+		print(addTags(INFO_TAG, string.format(template, ...)))
+	end
+end
+
+function Log.debug(template, ...)
+	if getLogLevel() >= Level.Debug then
+		print(addTags(DEBUG_TAG, string.format(template, ...)))
+	end
+end
+
+function Log.warn(template, ...)
+	if getLogLevel() >= Level.Warning then
+		warn(addTags(WARN_TAG, string.format(template, ...)))
+	end
+end
+
+return Log
\ No newline at end of file
diff --git a/plugin/log/init.spec.lua b/plugin/log/init.spec.lua
new file mode 100644
index 00000000..ff7e8e3b
--- /dev/null
+++ b/plugin/log/init.spec.lua
@@ -0,0 +1,5 @@
+return function()
+	it("should load", function()
+		require(script.Parent)
+	end)
+end
\ No newline at end of file
diff --git a/plugin/src/ApiContext.lua b/plugin/src/ApiContext.lua
index 0b6a7531..930ca2f3 100644
--- a/plugin/src/ApiContext.lua
+++ b/plugin/src/ApiContext.lua
@@ -1,9 +1,8 @@
 local Promise = require(script.Parent.Parent.Promise)
+local Http = require(script.Parent.Parent.Http)
 
 local Config = require(script.Parent.Config)
 local Version = require(script.Parent.Version)
-local Http = require(script.Parent.Http)
-local HttpError = require(script.Parent.HttpError)
 
 local ApiContext = {}
 ApiContext.__index = ApiContext
@@ -143,7 +142,7 @@ function ApiContext:retrieveMessages()
 	local function sendRequest()
 		return Http.get(url)
 			:catch(function(err)
-				if err.type == HttpError.Error.Timeout then
+				if err.type == Http.Error.Kind.Timeout then
 					return sendRequest()
 				end
 
diff --git a/plugin/src/Components/RojoFooter.lua b/plugin/src/Components/RojoFooter.lua
index 767e82bf..babc1a78 100644
--- a/plugin/src/Components/RojoFooter.lua
+++ b/plugin/src/Components/RojoFooter.lua
@@ -8,8 +8,6 @@ local Version = require(Plugin.Version)
 local Assets = require(Plugin.Assets)
 local Theme = require(Plugin.Theme)
 
-local FitText = require(Plugin.Components.FitText)
-
 local e = Roact.createElement
 
 local RojoFooter = Roact.Component:extend("RojoFooter")
diff --git a/plugin/src/Logging.lua b/plugin/src/Logging.lua
index 4b4f90af..398568ad 100644
--- a/plugin/src/Logging.lua
+++ b/plugin/src/Logging.lua
@@ -1,50 +1,2 @@
-local DevSettings = require(script.Parent.DevSettings)
-
-local Level = {
-	Error = 0,
-	Warning = 1,
-	Info = 2,
-	Trace = 3,
-}
-
-local testLogLevel = nil
-
-local function getLogLevel()
-	if testLogLevel ~= nil then
-		return testLogLevel
-	end
-
-	return DevSettings:getLogLevel()
-end
-
-local function addTags(tag, message)
-	return tag .. message:gsub("\n", "\n" .. tag)
-end
-
-local INFO_TAG = (" "):rep(15) .. "[Rojo-Info] "
-local TRACE_TAG = (" "):rep(15) .. "[Rojo-Trace] "
-local WARN_TAG = "[Rojo-Warn] "
-
-local Log = {}
-
-Log.Level = Level
-
-function Log.trace(template, ...)
-	if getLogLevel() >= Level.Trace then
-		print(addTags(TRACE_TAG, string.format(template, ...)))
-	end
-end
-
-function Log.info(template, ...)
-	if getLogLevel() >= Level.Info then
-		print(addTags(INFO_TAG, string.format(template, ...)))
-	end
-end
-
-function Log.warn(template, ...)
-	if getLogLevel() >= Level.Warning then
-		warn(addTags(WARN_TAG, string.format(template, ...)))
-	end
-end
-
-return Log
\ No newline at end of file
+-- TODO: Remove this once all references have been updated.
+return require(script.Parent.Parent.Log)
\ No newline at end of file
diff --git a/plugin/src/Reconciler.lua b/plugin/src/Reconciler.lua
index 66d41fe6..0e626bef 100644
--- a/plugin/src/Reconciler.lua
+++ b/plugin/src/Reconciler.lua
@@ -161,7 +161,10 @@ function Reconciler:__reify(virtualInstancesById, id, parent)
 	end)
 
 	if not ok then
-		error(("Couldn't create an Instance of type %q, a child of %s"):format(virtualInstance.ClassName, parent:GetFullName()))
+		error(("Couldn't create an Instance of type %q, a child of %s"):format(
+			virtualInstance.ClassName,
+			parent:GetFullName()
+		))
 	end
 
 	for key, value in pairs(virtualInstance.Properties) do
diff --git a/plugin/src/init.server.lua b/plugin/src/init.server.lua
index 3660d420..dce195ef 100644
--- a/plugin/src/init.server.lua
+++ b/plugin/src/init.server.lua
@@ -2,6 +2,14 @@ if not plugin then
 	return
 end
 
+local Log = require(script.Parent.Log)
+
+local DevSettings = require(script.DevSettings)
+
+Log.setLogLevelThunk(function()
+	return DevSettings:getLogLevel()
+end)
+
 local Roact = require(script.Parent.Roact)
 
 local Config = require(script.Config)
diff --git a/plugin/testBootstrap.server.lua b/plugin/testBootstrap.server.lua
index 81ed88b2..12f6b5f0 100644
--- a/plugin/testBootstrap.server.lua
+++ b/plugin/testBootstrap.server.lua
@@ -12,7 +12,7 @@ if setDevSettings then
 	DevSettings:createTestSettings()
 end
 
-TestEZ.TestBootstrap:run({Rojo.Plugin})
+TestEZ.TestBootstrap:run({ Rojo.Plugin, Rojo.Http, Rojo.Log })
 
 if setDevSettings then
 	DevSettings:resetValues()