Skip to content
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

Not parsing combined diffs correctly #23

Open
ishepard opened this issue May 2, 2019 · 6 comments
Open

Not parsing combined diffs correctly #23

ishepard opened this issue May 2, 2019 · 6 comments

Comments

@ishepard
Copy link

ishepard commented May 2, 2019

Hi @cscorley !
I was looking for a tool to parse combined diff to use inside my tool (https://github.com/ishepard/pydriller), and ended up here. In your documentation you state you can parse the combined diff (-c or --cc option), however I am not able to obtain complete results. Let's make an example. I have this combined diff:

cc5b002a5140e2d60184de42554a8737981c846c
diff --cc tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs
index aaad4c4a,8bf42fc3..4979ab7d
--- a/tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs
+++ b/tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs
@@@ -36,11 -36,9 +37,11 @@@ namespace Microsoft.AspNet.SignalR.Test
  
                  hubConnection.Start(host.Transport).Wait();
  
-                 proxy.Invoke("Send", "hello").Wait();
+                 proxy.InvokeWithTimeout("Send", "hello");
  
 -                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(5)));
 +                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(10)));
 +
 +                hubConnection.Stop();
              }
          }
  
@@@ -65,9 -63,9 +66,9 @@@
  
                  hubConnection.Start(host.Transport).Wait();
  
-                 proxy.Invoke("Send", "hello").Wait();
+                 proxy.InvokeWithTimeout("Send", "hello");
  
 -                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(5)));
 +                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(10)));
              }
          }
  
diff --cc tests/Microsoft.AspNet.SignalR.FunctionalTests/Server/Hubs/HubFacts.cs
index d153740f,4bdad4db..393a1ebf
--- a/tests/Microsoft.AspNet.SignalR.FunctionalTests/Server/Hubs/HubFacts.cs
+++ b/tests/Microsoft.AspNet.SignalR.FunctionalTests/Server/Hubs/HubFacts.cs
@@@ -297,8 -291,10 +298,8 @@@ namespace Microsoft.AspNet.SignalR.Test
                      Name = "David"
                  };
  
-                 var person1 = hub.Invoke<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>("ComplexType", person).Result;
+                 var person1 = hub.InvokeWithTimeout<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>("ComplexType", person);
                  var person2 = hub.GetValue<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>("person");
 -                JObject obj = ((dynamic)hub).person;
 -                var person3 = obj.ToObject<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>();
  
                  Assert.NotNull(person1);
                  Assert.NotNull(person2);

If I run:

for diff in whatthepatch.parse_patch(text):
    pprint(diff)

I obtain:

diff(header=header(index_path=None, old_path='--cc', old_version=None, new_path='tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs', new_version=None), changes=None, text='diff --cc tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs\nindex aaad4c4a,8bf42fc3..4979ab7d\n--- a/tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs\n+++ b/tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs\n@@@ -36,11 -36,9 +37,11 @@@ namespace Microsoft.AspNet.SignalR.Test\n  \n                  hubConnection.Start(host.Transport).Wait();\n  \n-                 proxy.Invoke("Send", "hello").Wait();\n+                 proxy.InvokeWithTimeout("Send", "hello");\n  \n -                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(5)));\n +                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(10)));\n +\n +                hubConnection.Stop();\n              }\n          }\n  \n@@@ -65,9 -63,9 +66,9 @@@\n  \n                  hubConnection.Start(host.Transport).Wait();\n  \n-                 proxy.Invoke("Send", "hello").Wait();\n+                 proxy.InvokeWithTimeout("Send", "hello");\n  \n -                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(5)));\n +                Assert.True(wh.WaitOne(TimeSpan.FromSeconds(10)));\n              }\n          }\n  \n')
diff(header=header(index_path=None, old_path='--cc', old_version=None, new_path='tests/Microsoft.AspNet.SignalR.FunctionalTests/Server/Hubs/HubFacts.cs', new_version=None), changes=None, text='diff --cc tests/Microsoft.AspNet.SignalR.FunctionalTests/Server/Hubs/HubFacts.cs\nindex d153740f,4bdad4db..393a1ebf\n--- a/tests/Microsoft.AspNet.SignalR.FunctionalTests/Server/Hubs/HubFacts.cs\n+++ b/tests/Microsoft.AspNet.SignalR.FunctionalTests/Server/Hubs/HubFacts.cs\n@@@ -297,8 -291,10 +298,8 @@@ namespace Microsoft.AspNet.SignalR.Test\n                      Name = "David"\n                  };\n  \n-                 var person1 = hub.Invoke<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>("ComplexType", person).Result;\n+                 var person1 = hub.InvokeWithTimeout<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>("ComplexType", person);\n                  var person2 = hub.GetValue<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>("person");\n -                JObject obj = ((dynamic)hub).person;\n -                var person3 = obj.ToObject<SignalR.Samples.Hubs.DemoHub.DemoHub.Person>();\n  \n                  Assert.NotNull(person1);\n                  Assert.NotNull(person2);\n')

As you can see, old_path is "--cc", that doesn't make sense, and changes are always None.

@ishepard
Copy link
Author

ishepard commented May 2, 2019

Actually, I just noticed that in the documentation you say "context diff", not "combined".
At this point I think this is more of a feature request than an issue :)

@cscorley
Copy link
Owner

cscorley commented May 2, 2019

Hey! Big fan of the pydriller work, been following the research output for a bit. :)

Yes, this is something I think we can add from this example file. I was planning on doing a new release soon anyway. Do you have any example of what you'd expect the output structure to be? Currently each line change would just all be collapsed into one list of changes, but I'm not sure if having the source it came from would be useful or not for your use case.

@ishepard
Copy link
Author

ishepard commented May 3, 2019

Awesome 😄 what I'd need for PyDriller is: old-path, new-path, modification type (added, deleted, etc.), and diff. So it shouldn't be too difficult to get this. We can start from these 4 things and maybe in the future we also include changes.

@cscorley
Copy link
Owner

Apologies for the slow work. Not a lot of free weekends this summer. I promise I started this, but got stuck :-)

@ishepard
Copy link
Author

Oh no worries! I know the feeling 😄

@cscorley
Copy link
Owner

Didn't get to include this in the 0.0.6 release; primary problem is I think there should be a few new format fields that other formats don't need and the refactor got deep. Did want to make sure the other changes got out to the public though since it's been ... ⌚️ 👀 ... 2.5 years

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants