-w/--watch option #104
-w/--watch option #104
Conversation
yjroot
force-pushed
the
watch-option
branch
3 times, most recently
from
26aa4be to
cbe7851
Compare
src/Nirum/Cli.hs
Outdated
| , watching :: Bool | ||
| , building :: TFlag | ||
| , changed :: TFlag | ||
| } |
There was a problem hiding this comment.
@yjroot @admire93 위쪽에 있는 Options와도 많이 겹치는 것 같은데, 둘이 어떻게 다른 거죠? 공통된 것은 합칠 방법 없을까요?
src/Nirum/Cli.hs
Outdated
| } | ||
|
|
||
| debounceDelay :: Int | ||
| debounceDelay = 1 * 1000 * 1000 |
There was a problem hiding this comment.
단위가 어떻게 되나요? 단위가 드러났으면 좋겠습니다. 상수명 뒤에 단위를 붙이거나, 아니면 type Millisecond = Int 뭐 이런 식으로 type alias한 다음에 해당 타입으로 지정하거나…
src/Nirum/Cli.hs
Outdated
| Left (CompileError errors) -> | ||
| forM_ (M.toList errors) $ \ (filePath, compileError) -> | ||
| die [qq|error: $filePath: $compileError|] | ||
| tryDie options [qq|error: $filePath: $compileError|] |
There was a problem hiding this comment.
이 함수 안에서 반복해서 tryDie options ...를 호출하고 있는데, 차라리 이 함수의 where에 try' = tryDie options 만들어 두고 그걸 쓰게 하는 게 낫지 않을까요.
src/Nirum/Cli.hs
Outdated
| options@AppOptions { building = building' | ||
| , changed = changed' | ||
| } | ||
| event |
There was a problem hiding this comment.
AppOptions의 닫는 중괄호 위치가 여는 중괄호보다 왼쪽에 있네요. 그리고 onFileChanged 함수의 인자들도 밑에 있는 가드와 같은 위치에 있어서 좀 핫갈리는 것 같습니다. 인자들은 차라리 onFileChanged 바로 뒤에 위치시키는 건 어떨까요? 이런 식으로:
onFileChanged :: AppOptions -> Event -> IO ()
onFileChanged options@AppOptions { building = building'
, changed = changed'
}
event
src/Nirum/Cli.hs
Outdated
| (T.pack $ targetOption opts) | ||
| watch' | ||
| building' | ||
| changed' |
There was a problem hiding this comment.
이 정도면 필드가 꽤 많으니 그냥 레코드 문법을 쓰는 게 좋을 것 같습니다.
src/Nirum/Cli.hs
Outdated
| @@ -184,5 +258,8 @@ main = do | |||
| OPT.strOption | |||
| (OPT.long "target" <> OPT.short 't' <> OPT.metavar "TARGET" <> | |||
| OPT.help "Target language name") <*> | |||
| OPT.switch | |||
| (OPT.long "watch" <> OPT.short 'w' <> | |||
| OPT.help "Watches files for changes and rebuilds") <*> | |||
There was a problem hiding this comment.
3인칭 단수형으로 쓰지 말고 그냥 명령형으로 쓰면 될 것 같습니다. (다들 그렇게 쓰는 것 같네요.)
yjroot
force-pushed
the
watch-option
branch
2 times, most recently
from
7b7a50b to
7cb2879
Compare
src/Nirum/Cli.hs
Outdated
| import GHC.Exts (IsList (toList)) | ||
|
|
||
| import qualified Data.ByteString as B | ||
| import qualified Data.Map.Strict as M | ||
| import qualified Data.Set as S | ||
| import qualified Data.Text as T | ||
| import Control.Concurrent (threadDelay) |
There was a problem hiding this comment.
Control.Concurrent는 표준 라이브러리니까 위쪽으로 올려주세요.
src/Nirum/Cli.hs
Outdated
| import GHC.Exts (IsList (toList)) | ||
|
|
||
| import qualified Data.ByteString as B | ||
| import qualified Data.Map.Strict as M | ||
| import qualified Data.Set as S | ||
| import qualified Data.Text as T | ||
| import Control.Concurrent (threadDelay) | ||
| import Control.Concurrent.STM (atomically, newTVar, readTVar, TVar, writeTVar) | ||
| import Control.Monad (forM_, forever, when) |
There was a problem hiding this comment.
Control.Monad는 표준 라이브러리니까 위쪽으로 올려주세요.
src/Nirum/Cli.hs
Outdated
| import System.FilePath (takeDirectory, (</>)) | ||
| import System.FilePath (takeDirectory, takeExtension, (</>)) | ||
| import System.FSNotify | ||
| import System.IO |
There was a problem hiding this comment.
System.IO는 표준 라이브러리니까 위쪽으로 올려주세요.
dahlia
added
the
cmp:frontend
Fixes #91.