Skip to content

Commit

Permalink
Format returns 0 when file is not formatted (#2181)
Browse files Browse the repository at this point in the history
`format` command now returns code `0` most of the time.

It will return `1` when:
* some error occur, so can not format
* file is unformatted and `--check` option is used
* One or more files are not formatted in a Juvix project.

- Fixes #2171
  • Loading branch information
vrom911 authored Jun 7, 2023
1 parent 4c0e0b1 commit 8e59624
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 13 deletions.
13 changes: 12 additions & 1 deletion app/Commands/Format.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ data FormatTarget
= TargetFile
| TargetDir
| TargetStdin
deriving stock (Eq)

runCommand :: forall r. Members '[Embed IO, App, Resource, Files] r => FormatOptions -> Sem r ()
runCommand opts = do
Expand All @@ -44,7 +45,17 @@ runCommand opts = do
"Use the --help option to display more usage information."
]
return FormatResultFail
when (res == FormatResultFail) (embed (exitWith (ExitFailure 1)))
let exitFail :: IO a
exitFail = exitWith (ExitFailure 1)
case res of
FormatResultFail -> embed exitFail
FormatResultNotFormatted ->
{- use exit code 1 for
* unformatted files when using --check
* when running the formatter on a Juvix project
-}
when (opts ^. formatCheck || target == TargetDir) (embed exitFail)
FormatResultOK -> pure ()

renderModeFromOptions :: FormatTarget -> FormatOptions -> FormattedFileInfo -> FormatRenderMode
renderModeFromOptions target opts formattedInfo
Expand Down
25 changes: 17 additions & 8 deletions src/Juvix/Formatter.hs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,22 @@ makeSem ''ScopeEff

data FormatResult
= FormatResultOK
| FormatResultNotFormatted
| FormatResultFail
deriving stock (Eq)

instance Semigroup FormatResult where
FormatResultFail <> _ = FormatResultFail
_ <> FormatResultFail = FormatResultFail
FormatResultNotFormatted <> _ = FormatResultNotFormatted
_ <> FormatResultNotFormatted = FormatResultNotFormatted
_ <> _ = FormatResultOK

instance Monoid FormatResult where
mempty = FormatResultOK

combineResults :: [FormatResult] -> FormatResult
combineResults rs
| FormatResultFail `elem` rs = FormatResultFail
| otherwise = FormatResultOK
combineResults = mconcat

ansiPlainText :: NonEmpty AnsiText -> Text
ansiPlainText = T.concat . toList . fmap toPlainText
Expand All @@ -43,11 +52,11 @@ formattedFileInfoContentsText = to infoToPlainText

-- | Format a single Juvix file.
--
-- If the file requires formatting then the function returns FormatResultFail
-- If the file requires formatting then the function returns 'FormatResultNotFormatted'
-- and outputs a FormattedFileInfo containing the formatted contents of the file.
--
-- If the file does not require formatting then the function returns
-- FormatResultOK and nothing is output.
-- 'FormatResultOK' and nothing is output.
format ::
forall r.
Members '[ScopeEff, Files, Output FormattedFileInfo] r =>
Expand All @@ -62,12 +71,12 @@ format p = do
--
-- Format all files in the Juvix project containing the passed directory.
--
-- If any file requires formatting then the function returns FormatResultFail.
-- If any file requires formatting then the function returns 'FormatResultNotFormatted'
-- This function also outputs a FormattedFileInfo (containing the formatted
-- contents of a file) for every file in the project that requires formatting.
--
-- If all files in the project are already formatted then the function returns
-- FormatResultOK and nothing is output.
-- 'FormatResultOK' and nothing is output.
--
-- NB: This function does not traverse into Juvix sub-projects, i.e into
-- subdirectories that contain a juvix.yaml file.
Expand Down Expand Up @@ -122,7 +131,7 @@ formatResultFromContents originalContents mfc filepath =
_formattedFileInfoContentsAnsi = formattedContents
}
)
return FormatResultFail
return FormatResultNotFormatted
| otherwise -> return FormatResultOK
Nothing ->
return FormatResultOK
Expand Down
3 changes: 2 additions & 1 deletion test/Formatter/Positive.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ makeFormatTest' Scope.PosTest {..} =
d <- runM $ runError $ runOutputList @FormattedFileInfo $ runScopeEffIO $ runFilesIO $ format file'
case d of
Right (_, FormatResultOK) -> return ()
Right (_, FormatResultFail) -> assertFailure ("File: " <> show file' <> " is not formatted")
Right (_, FormatResultNotFormatted) -> assertFailure ("File: " <> show file' <> " is not formatted")
Right (_, FormatResultFail) -> assertFailure ("File: " <> show file' <> " is failed to format")
Left {} -> assertFailure ("Error: ")
}

Expand Down
6 changes: 3 additions & 3 deletions tests/smoke/Commands/format.smoke.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ tests:
juvix format Foo.juvix
stdout:
contains: module Foo;
exit-status: 1
exit-status: 0

- name: format-unformatted-file-check-no-stdout
command:
Expand Down Expand Up @@ -168,7 +168,7 @@ tests:
main : Nat;
main := 5;
exit-status: 1
exit-status: 0

- name: format-stdin-file-does-not-exist
command:
Expand Down Expand Up @@ -207,7 +207,7 @@ tests:
main : Nat;
main := 5;
exit-status: 1
exit-status: 0

- name: format-no-stdin-no-file-name
command:
Expand Down

0 comments on commit 8e59624

Please sign in to comment.