From 58d99a76c26b1ce4cffcdc5378cac58af541c026 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 23 Sep 2016 15:46:52 -0700 Subject: [PATCH 01/10] Accept --no-index to compare two paths --- src/Arguments.hs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Arguments.hs b/src/Arguments.hs index 0d9f058b3..38944587d 100644 --- a/src/Arguments.hs +++ b/src/Arguments.hs @@ -11,6 +11,7 @@ data Arguments = Arguments { maybeShas :: Both (Maybe P.String), maybeTimeout :: Maybe Float, output :: Maybe FilePath, + noIndex :: Bool, filepaths :: [FilePath] } deriving (Show) @@ -20,4 +21,5 @@ args sha1 sha2 filePaths format = Arguments { format = format , filepaths = filePaths , maybeTimeout = Just 10.0 , output = Nothing + , noIndex = False } From b8ad29fbca6e950ebebdd654716f380548ace1b9 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Tue, 27 Sep 2016 14:56:29 -0700 Subject: [PATCH 02/10] Custom argument parsing for sha ranges and file paths --- src/Arguments.hs | 49 +++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/Arguments.hs b/src/Arguments.hs index 38944587d..e61df8d51 100644 --- a/src/Arguments.hs +++ b/src/Arguments.hs @@ -1,25 +1,40 @@ -module Arguments (Arguments(..), args) where +module Arguments (Arguments(..), ExtraArg(..), args, filePathsFromArgs, maybeShasFromArgs) where import Data.Functor.Both -import qualified Prelude as P +import Data.Monoid import Prelude import qualified Renderer as R +data ExtraArg = ShaPair (Both (Maybe String)) | FileArg FilePath deriving (Show) + -- | The command line arguments to the application. -data Arguments = Arguments { - format :: R.Format, - maybeShas :: Both (Maybe P.String), - maybeTimeout :: Maybe Float, - output :: Maybe FilePath, - noIndex :: Bool, - filepaths :: [FilePath] } - deriving (Show) +data Arguments = Arguments + { format :: R.Format + , maybeTimeout :: Maybe Float + , output :: Maybe FilePath + , noIndex :: Bool + , extraArgs :: [ExtraArg] + } deriving (Show) args :: String -> String -> [String] -> R.Format -> Arguments -args sha1 sha2 filePaths format = Arguments { format = format - , maybeShas = Just <$> both sha1 sha2 - , filepaths = filePaths - , maybeTimeout = Just 10.0 - , output = Nothing - , noIndex = False - } +args sha1 sha2 filePaths format = Arguments + { format = format + , maybeTimeout = Just 10.0 + , output = Nothing + , noIndex = False + , extraArgs = [ShaPair (Just <$> both sha1 sha2)] <> (FileArg <$> filePaths) + } + +filePathsFromArgs :: Arguments -> [FilePath] +filePathsFromArgs Arguments{..} = go extraArgs + where + go [] = [] + go (FileArg x:xs) = x : go xs + go (_:xs) = go xs + +maybeShasFromArgs :: Arguments -> Both (Maybe String) +maybeShasFromArgs Arguments{..} = go extraArgs + where + go [] = both Nothing Nothing + go (ShaPair x:_) = x + go (_:xs) = go xs From fb8849ecaefe64417376b06ea452b66558c7abf0 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Thu, 29 Sep 2016 15:56:39 -0700 Subject: [PATCH 03/10] Refactor argument parsing --- src/Arguments.hs | 90 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 20 deletions(-) diff --git a/src/Arguments.hs b/src/Arguments.hs index e61df8d51..ef56d7ac4 100644 --- a/src/Arguments.hs +++ b/src/Arguments.hs @@ -1,40 +1,90 @@ -module Arguments (Arguments(..), ExtraArg(..), args, filePathsFromArgs, maybeShasFromArgs) where +module Arguments (Arguments(..), ExtraArg(..), programArguments, args) where import Data.Functor.Both -import Data.Monoid +import Data.Maybe +import Data.Text +import Prologue hiding ((<>)) import Prelude +import System.Environment +import System.Directory +import System.IO.Error (IOError) + import qualified Renderer as R data ExtraArg = ShaPair (Both (Maybe String)) | FileArg FilePath deriving (Show) -- | The command line arguments to the application. -data Arguments = Arguments +data Arguments = + -- | Arguments for optparse-applicative + CmdLineArguments { format :: R.Format , maybeTimeout :: Maybe Float , output :: Maybe FilePath , noIndex :: Bool , extraArgs :: [ExtraArg] + } + -- | Arguments actually used by the program (includes command line, environment, and default args). + | Arguments + { gitDir :: FilePath + , alternateObjectDirs :: [Text] + , format :: R.Format + , timeoutInMicroseconds :: Int + , output :: Maybe FilePath + , noIndex :: Bool + , shaRange :: Both (Maybe String) + , filePaths :: [FilePath] } deriving (Show) -args :: String -> String -> [String] -> R.Format -> Arguments -args sha1 sha2 filePaths format = Arguments - { format = format - , maybeTimeout = Just 10.0 +-- | Returns Arguments for the program from parsed command line arguments. +programArguments :: Arguments -> IO Arguments +programArguments args@Arguments{..} = pure args +programArguments CmdLineArguments{..} = do + pwd <- getCurrentDirectory + gitDir <- fromMaybe pwd <$> lookupEnv "GIT_DIR" + eitherObjectDirs <- try $ parseObjectDirs . toS <$> getEnv "GIT_ALTERNATE_OBJECT_DIRECTORIES" + let alternateObjectDirs = case (eitherObjectDirs :: Either IOError [Text]) of + (Left _) -> [] + (Right objectDirs) -> objectDirs + pure Arguments + { gitDir = gitDir + , alternateObjectDirs = alternateObjectDirs + , format = format + , timeoutInMicroseconds = maybe defaultTimeout toMicroseconds maybeTimeout + , output = output + , noIndex = noIndex + , shaRange = fetchShas extraArgs + , filePaths = fetchPaths extraArgs + } + where + fetchPaths :: [ExtraArg] -> [FilePath] + fetchPaths [] = [] + fetchPaths (FileArg x:xs) = x : fetchPaths xs + fetchPaths (_:xs) = fetchPaths xs + + fetchShas :: [ExtraArg] -> Both (Maybe String) + fetchShas [] = both Nothing Nothing + fetchShas (ShaPair x:_) = x + fetchShas (_:xs) = fetchShas xs + +-- | Quickly assemble an Arguments data record with defaults. +args :: FilePath -> String -> String -> [String] -> R.Format -> Arguments +args gitDir sha1 sha2 filePaths format = Arguments + { gitDir = gitDir + , alternateObjectDirs = [] + , format = format + , timeoutInMicroseconds = defaultTimeout , output = Nothing , noIndex = False - , extraArgs = [ShaPair (Just <$> both sha1 sha2)] <> (FileArg <$> filePaths) + , shaRange = Just <$> both sha1 sha2 + , filePaths = filePaths } -filePathsFromArgs :: Arguments -> [FilePath] -filePathsFromArgs Arguments{..} = go extraArgs - where - go [] = [] - go (FileArg x:xs) = x : go xs - go (_:xs) = go xs +-- | 7 seconds +defaultTimeout :: Int +defaultTimeout = 7 * 1000000 -maybeShasFromArgs :: Arguments -> Both (Maybe String) -maybeShasFromArgs Arguments{..} = go extraArgs - where - go [] = both Nothing Nothing - go (ShaPair x:_) = x - go (_:xs) = go xs +toMicroseconds :: Float -> Int +toMicroseconds num = floor $ num * 1000000 + +parseObjectDirs :: Text -> [Text] +parseObjectDirs = split (== ':') From d03a0c78f7d715a553533ec295071b81e1806093 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Thu, 29 Sep 2016 16:22:45 -0700 Subject: [PATCH 04/10] Better to have these as their own data records --- file1.js | 1 + file2.js | 1 + src/Arguments.hs | 30 ++++++++++++++---------------- 3 files changed, 16 insertions(+), 16 deletions(-) create mode 100644 file1.js create mode 100644 file2.js diff --git a/file1.js b/file1.js new file mode 100644 index 000000000..c1c6922d1 --- /dev/null +++ b/file1.js @@ -0,0 +1 @@ +return true; diff --git a/file2.js b/file2.js new file mode 100644 index 000000000..1f328b3bd --- /dev/null +++ b/file2.js @@ -0,0 +1 @@ +return false; diff --git a/src/Arguments.hs b/src/Arguments.hs index ef56d7ac4..c6dc820de 100644 --- a/src/Arguments.hs +++ b/src/Arguments.hs @@ -1,4 +1,4 @@ -module Arguments (Arguments(..), ExtraArg(..), programArguments, args) where +module Arguments (Arguments(..), CmdLineOptions(..), ExtraArg(..), programArguments, args) where import Data.Functor.Both import Data.Maybe @@ -13,18 +13,17 @@ import qualified Renderer as R data ExtraArg = ShaPair (Both (Maybe String)) | FileArg FilePath deriving (Show) --- | The command line arguments to the application. -data Arguments = - -- | Arguments for optparse-applicative - CmdLineArguments - { format :: R.Format +-- | The command line options to the application (arguments for optparse-applicative). +data CmdLineOptions = CmdLineOptions + { outputFormat :: R.Format , maybeTimeout :: Maybe Float - , output :: Maybe FilePath - , noIndex :: Bool + , outputFilePath :: Maybe FilePath + , noIndexMode :: Bool , extraArgs :: [ExtraArg] } - -- | Arguments actually used by the program (includes command line, environment, and default args). - | Arguments + +-- | Arguments for the program (includes command line, environment, and defaults). +data Arguments = Arguments { gitDir :: FilePath , alternateObjectDirs :: [Text] , format :: R.Format @@ -36,9 +35,8 @@ data Arguments = } deriving (Show) -- | Returns Arguments for the program from parsed command line arguments. -programArguments :: Arguments -> IO Arguments -programArguments args@Arguments{..} = pure args -programArguments CmdLineArguments{..} = do +programArguments :: CmdLineOptions -> IO Arguments +programArguments CmdLineOptions{..} = do pwd <- getCurrentDirectory gitDir <- fromMaybe pwd <$> lookupEnv "GIT_DIR" eitherObjectDirs <- try $ parseObjectDirs . toS <$> getEnv "GIT_ALTERNATE_OBJECT_DIRECTORIES" @@ -48,10 +46,10 @@ programArguments CmdLineArguments{..} = do pure Arguments { gitDir = gitDir , alternateObjectDirs = alternateObjectDirs - , format = format + , format = outputFormat , timeoutInMicroseconds = maybe defaultTimeout toMicroseconds maybeTimeout - , output = output - , noIndex = noIndex + , output = outputFilePath + , noIndex = noIndexMode , shaRange = fetchShas extraArgs , filePaths = fetchPaths extraArgs } From e18da624da03c9820ae9adbca0a029d66d1f460f Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Thu, 29 Sep 2016 16:25:26 -0700 Subject: [PATCH 05/10] Whoops, remove my test files --- file1.js | 1 - file2.js | 1 - 2 files changed, 2 deletions(-) delete mode 100644 file1.js delete mode 100644 file2.js diff --git a/file1.js b/file1.js deleted file mode 100644 index c1c6922d1..000000000 --- a/file1.js +++ /dev/null @@ -1 +0,0 @@ -return true; diff --git a/file2.js b/file2.js deleted file mode 100644 index 1f328b3bd..000000000 --- a/file2.js +++ /dev/null @@ -1 +0,0 @@ -return false; From 46353c763ff4875ab95a64ea755d6b33ff25e36b Mon Sep 17 00:00:00 2001 From: Rick Winfrey Date: Fri, 30 Sep 2016 15:14:17 -0500 Subject: [PATCH 06/10] Add 2016-09-30 weekly notes --- weekly/2016-09-30.md | 64 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 weekly/2016-09-30.md diff --git a/weekly/2016-09-30.md b/weekly/2016-09-30.md new file mode 100644 index 000000000..7c6a7f630 --- /dev/null +++ b/weekly/2016-09-30.md @@ -0,0 +1,64 @@ +#09-30-2018 + +**What went well?** + +@tclem: Quiet week with everyone in Japan. Was heads down in parsing issues, identifying determiners. Tackled some other harder issues (ongoing now around tooling and command line parsing). + +@joshvera: ICFP was spectacular. There's a lot to write down, and ideas that could potentially apply to future features or techniques in general that would be useful. Testing RWS has gone well. Issue came up with small RWS / SES diff ordering. + +@rewinfrey: ICFP also amazing. This week small issues and started looking at keeping parent contexts for nested contexts (and have a RFC out). + +@pengwynn: Good to interface with everyone, and to see if we provide further support with platform and integrating with dotcom. + +**What were the challenges?** + +@tclem: Challenging being totally on my own a couple weeks into the project. Some issues came up that required some heavy investigation (like monad transformers). + +@joshvera: Coming back has been surprisingly difficult to have normal hours (going to Japan was easy). RWS bug with ordering terms is concerning, but should be manageable. Some PRs on master fail and think they are problems that come up in our property tests rarely (should look back and get those seeds so we can reproduce them). We have to make our diffs commute, so that if we're producing the same diff terms, they should always be in the same order. Sometimes surrounding context will change or alter diffs. + +@rewinfrey: I'm not looking at RWS so things are going well :p + +**What did you learn?** + +@tclem: Studying folds and algebraic data types. Learning about binary tree operations. Understanding of the entire architecture stack. + +@joshvera: Learned a lot at ICFP - specifically about effects and coeffects (reifying as types in a language). Effects do something to a context, coeffects reach into and look at a context. For example, a high security program that communicates with a low security program could allow the low security program to use a coeffect to understand something about what happened in the high security program, but wouldn't reveal the sensitive information. + +@rewinfrey: Learned a lot about freer monad and effects in general. Another big take away was homotopy type theory. + +---- + +@pengwynn: So I'd like to ask what are we missing on the team? + +@joshvera: The help we get from dotcom (mclark and brianmario) is very helpful so we don't have to spend as much time diving into dotcom. We haven't received a lot of design attention - have a couple technical reasons why semantic diff is currently disabled. Would like to follow up with Fabian on design next week. Would like to find people with more Haskell experience, or with stronger background in diffing in general outside of the context of version control. One of the bigger technical areas of work we will have to do is writing parsers and ensuring they are correct. + +@pengwynn: How do we leverage data to make autonomous coding a reality? As we move closer to 2017 what's the team make up look like? + +@joshvera: We're getting to a point where semantic diffing is stable. Figuring out a way to generate a corpus from the information we have would be a really hard technical solution, but would pay back in dividends. Things like semantic merging. Also Rob has ideas about how we would compare structures across different languages, and code navigation (semantic click through to definition). What are the most useful features for people we could develop immediately? It would be good to have a stronger interface with product people than we currently do. + +@tclem: We could have a dedicated effort on parsing. + +@pengwynn: Is that a polyglot or Haskeller? + +@joshvera: That's someone like Max (Max Brunsfeld) at the moment -- but in general that means a polyglot. + +@tclem: I would +1 for fabian or product design. Having someone that can help us understand the context of the future of PR's, and where that general set of features are going. + +@pengwynn: What would it take once we launch diff summaries to go from 2 languages to 25? If that's one hire we can make that happen. Want to +poll the group to determine what our holes are and what we should look for when hiring for the next teammate. + +@tclem: Looking at parsers is a good thing. + +@joshvera: Getting someone to be able to write parsers from a specification (which is researched). Taking existing stuff and putting it into a system that works for us. Also integrating with existing compilers, so we don't have to manually update our parsers every time a language change would be very beneficial. Automating that problem away would be beneficial, too. Hooking into different compilers would be something that would save us effort and energy in the future. + +@pengwynn: This is awesome, and spurs some conversations with JD's team. + +---- + +@rewinfrey: Where are we with getting diff summaries staff shipped and also out to gen pop? Would it be good to have a conversation about that early next week to clarify the direction for prioritization? + +@joshvera: RWS and platform interface issues are the two main things preventing us from staff shipping again. Also want to talk with product design to take existing feedback and adjust accordingly. There are a few things in the summer eyes milestone that I think would be most useful, but maybe those aren't what product thinks is most useful. There could be more tiny bugs that only come up in certain contexts that we might not be aware of since we only do 100 property tests. + +@tclem: It feels like we're close, and that it's really product and design and incorporating diff summaries into the new focus on code review. + +@joshvera: I'm planning to talk with Fabian early next week to sync up on design. From 60e86181aab236ddb937252c5d15dc2300e85f83 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 30 Sep 2016 13:51:45 -0700 Subject: [PATCH 07/10] New DiffMode datatype instead of noIndex Bool --- src/Arguments.hs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/Arguments.hs b/src/Arguments.hs index c6dc820de..d8eabdcde 100644 --- a/src/Arguments.hs +++ b/src/Arguments.hs @@ -1,4 +1,4 @@ -module Arguments (Arguments(..), CmdLineOptions(..), ExtraArg(..), programArguments, args) where +module Arguments (Arguments(..), CmdLineOptions(..), DiffMode(..), ExtraArg(..), programArguments, args) where import Data.Functor.Both import Data.Maybe @@ -11,14 +11,20 @@ import System.IO.Error (IOError) import qualified Renderer as R -data ExtraArg = ShaPair (Both (Maybe String)) | FileArg FilePath deriving (Show) +data ExtraArg = ShaPair (Both (Maybe String)) + | FileArg FilePath + deriving (Show) + +data DiffMode = PathDiff (Both FilePath) + | CommitDiff + deriving (Show) -- | The command line options to the application (arguments for optparse-applicative). data CmdLineOptions = CmdLineOptions { outputFormat :: R.Format , maybeTimeout :: Maybe Float , outputFilePath :: Maybe FilePath - , noIndexMode :: Bool + , noIndex :: Bool , extraArgs :: [ExtraArg] } @@ -29,7 +35,7 @@ data Arguments = Arguments , format :: R.Format , timeoutInMicroseconds :: Int , output :: Maybe FilePath - , noIndex :: Bool + , diffMode :: DiffMode , shaRange :: Both (Maybe String) , filePaths :: [FilePath] } deriving (Show) @@ -43,15 +49,19 @@ programArguments CmdLineOptions{..} = do let alternateObjectDirs = case (eitherObjectDirs :: Either IOError [Text]) of (Left _) -> [] (Right objectDirs) -> objectDirs + + let filePaths = fetchPaths extraArgs pure Arguments { gitDir = gitDir , alternateObjectDirs = alternateObjectDirs , format = outputFormat , timeoutInMicroseconds = maybe defaultTimeout toMicroseconds maybeTimeout , output = outputFilePath - , noIndex = noIndexMode + , diffMode = case (noIndex, filePaths) of + (True, [fileA, fileB]) -> PathDiff (both fileA fileB) + (_, _) -> CommitDiff , shaRange = fetchShas extraArgs - , filePaths = fetchPaths extraArgs + , filePaths = filePaths } where fetchPaths :: [ExtraArg] -> [FilePath] @@ -72,7 +82,7 @@ args gitDir sha1 sha2 filePaths format = Arguments , format = format , timeoutInMicroseconds = defaultTimeout , output = Nothing - , noIndex = False + , diffMode = CommitDiff , shaRange = Just <$> both sha1 sha2 , filePaths = filePaths } From 4f1a609bda121003dd159a0e812314a0b1c05a33 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 30 Sep 2016 14:09:30 -0700 Subject: [PATCH 08/10] Catch string identifiers and skip squoting --- src/DiffSummary.hs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/DiffSummary.hs b/src/DiffSummary.hs index 586a86562..28e47f586 100644 --- a/src/DiffSummary.hs +++ b/src/DiffSummary.hs @@ -111,6 +111,7 @@ toLeafInfos :: DiffInfo -> [Doc] toLeafInfos (LeafInfo "number" termName) = pure (squotes (toDoc termName)) toLeafInfos (LeafInfo "boolean" termName) = pure (squotes (toDoc termName)) toLeafInfos (LeafInfo "anonymous function" termName) = pure (toDoc termName) +toLeafInfos (LeafInfo cName@"string" termName) = pure (toDoc termName <+> toDoc cName) toLeafInfos LeafInfo{..} = pure (squotes (toDoc termName) <+> toDoc categoryName) toLeafInfos BranchInfo{..} = toLeafInfos =<< branches toLeafInfos err@ErrorInfo{} = pure (pretty err) From 45a12c54f0a336bd0a44ff509bb8d65f131ba9b5 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 30 Sep 2016 14:15:02 -0700 Subject: [PATCH 09/10] Fix testDiff in DiffSummarySpecs too --- test/DiffSummarySpec.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/DiffSummarySpec.hs b/test/DiffSummarySpec.hs index 39c7527f0..9f0a82509 100644 --- a/test/DiffSummarySpec.hs +++ b/test/DiffSummarySpec.hs @@ -28,7 +28,7 @@ literalInfo :: Record '[Category, Range] literalInfo = StringLiteral .: Range 1 2 .: RNil testDiff :: Diff (Syntax Text) (Record '[Category, Range]) -testDiff = free $ Free (pure arrayInfo :< Indexed [ free $ Pure (Insert (cofree $ literalInfo :< Leaf "a")) ]) +testDiff = free $ Free (pure arrayInfo :< Indexed [ free $ Pure (Insert (cofree $ literalInfo :< Leaf "\"a\"")) ]) testSummary :: DiffSummary DiffInfo testSummary = DiffSummary { patch = Insert (LeafInfo "string" "a"), parentAnnotation = Nothing } @@ -43,7 +43,7 @@ spec :: Spec spec = parallel $ do describe "diffSummaries" $ do it "outputs a diff summary" $ do - diffSummaries blobs testDiff `shouldBe` [ Right $ "Added the 'a' string" ] + diffSummaries blobs testDiff `shouldBe` [ Right $ "Added the \"a\" string" ] prop "equal terms produce identity diffs" $ \ a -> let term = defaultFeatureVectorDecorator (category . headF) (toTerm (a :: ArbitraryTerm Text (Record '[Category, Range]))) in From 3f7bb0a3ff100a525668f6a8f4d1515d4ce0a212 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 30 Sep 2016 16:08:19 -0700 Subject: [PATCH 10/10] Update atom build package config --- .atom-build.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.atom-build.yml b/.atom-build.yml index ff05c8224..52a715550 100644 --- a/.atom-build.yml +++ b/.atom-build.yml @@ -1,14 +1,12 @@ # Build configuration for https://atom.io/packages/build -cmd: stack build semantic-diff +cmd: stack build name: semantic-diff env: PATH: ~/.local/bin:~/Developer/Tools:~/Library/Haskell/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin targets: test: - cmd: stack build :semantic-diff-test + cmd: stack build :integration-test keymap: cmd-u - semantic-difftool: - cmd: stack build :semantic-difftool semantic-git-diff: cmd: stack build :semantic-git-diff errorMatch: