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

Think about better support for pretty-diffs #47

Open
LinqLover opened this issue Nov 7, 2021 · 0 comments
Open

Think about better support for pretty-diffs #47

LinqLover opened this issue Nov 7, 2021 · 0 comments

Comments

@LinqLover
Copy link
Contributor

I just stumbled into a situation where traditional diffs are easier to read than pretty diffs:

From:

installPreviewDependenciesForTests

	^ self installPreviewDependencies: {
		self depInbox: 'SUnit-ct.125'. "TestCase>>#runCaseWithoutTimeout for SandboxKernelTest"
		self depInbox: 'SUnit-ct.132'. "TestCase>>#assert:equals:description: with lazy descriptions"
		self depInbox: 'Kernel-ct.1407'. "Fixes Context >> #isPrimFailToken: for ProtoObjects"
	}

To:

installPreviewDependenciesForTests

	^ self installPreviewDependencies: {
		self depInbox: 'SUnit-ct.125'. "TestCase>>#runCaseWithoutTimeout for SandboxKernelTest"
		self depInbox: 'SUnit-ct.132'. "TestCase>>#assert:equals:description: with lazy descriptions"
	}

Traditional diff:

 installPreviewDependenciesForTests
 
 	^ self installPreviewDependencies: {
 		self depInbox: 'SUnit-ct.125'. "TestCase>>#runCaseWithoutTimeout for SandboxKernelTest"
 		self depInbox: 'SUnit-ct.132'. "TestCase>>#assert:equals:description: with lazy descriptions"
-		self depInbox: 'Kernel-ct.1407'. "Fixes Context >> #isPrimFailToken: for ProtoObjects"
 	}

Poppy-printed diff:

 installPreviewDependenciesForTests
 
-	^ self installPreviewDependencies: {self depInbox: 'SUnit-ct.125'. self depInbox: 'SUnit-ct.132'. self depInbox: 'Kernel-ct.1407'}
+	^ self installPreviewDependencies: {self depInbox: 'SUnit-ct.125'. self depInbox: 'SUnit-ct.132'}
 	"TestCase>>#runCaseWithoutTimeout for SandboxKernelTest"
-	"TestCase>>#assert:equals:description: with lazy descriptions"
-	"Fixes Context >> #isPrimFailToken: for ProtoObjects"
+	"TestCase>>#assert:equals:description: with lazy descriptions"

Of course, this is a combination of multiple issues (#8 - my personal most wanted issue :-)), but it also shows that hard-coded line-break intervals might be an imperfect strategy when creating diffs. Probably PoppyPrint should support a separate diff mode where two source codes are compared based on their ASTs first and a diff is generated afterward? Or would this be out of scope for PoppyPrint?

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

1 participant