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

feat: Add the stdlib_diff tool to compare gno and go standard libraries #1425

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

FloRichardAloeCorp
Copy link

As requested by the issue https://github.com/gnolang/gno/issues/1310, the aim of this pull request is to add a tool under the misc directory to compare the Gno and Go standard libraries.

The tool generates a differences report per package.

To test the tool, build it, and refer to the README for usage instructions. The tool can be used in both ways: comparing Go to Gno or comparing Gno to Go.

I'm questioning the relevance of adding unit tests to this tool. If you believe it needs some, just let me know!

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • [x ] No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • [x ] Provided any useful hints for running manual tests
  • [x ] Added new benchmarks to generated graphs, if any. More info here.

@FloRichardAloeCorp FloRichardAloeCorp requested a review from a team as a code owner December 8, 2023 10:21
@FloRichardAloeCorp FloRichardAloeCorp changed the title feat/stdlib-diff: Add the stdlib_diff tool to compare gno and go standard libraries feat: Add the stdlib_diff tool to compare gno and go standard libraries Dec 8, 2023
Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

Sorry for the wait, this is an initial to improve. Nice work, but you can improve.

Some functionality missing on the generated report:

  1. Formatting is still very funky. Use CSS to render tabs correctly; they should render as 4 spaces.

image

  1. Ideally, for the added/deleted lines, make the + and - symbols non selectable. This makes it so that if I select the lines in either side of the page I can copy and paste the code without copying the symbols.
  2. I believe the report's performance could be improved. Consider using the library I mention in the comments (diffmatchpatch) and parallelizing execution. You can parallelize execution by having a goroutine trying to first of all find all of the directories in the source and destination, and then sending off to a channel individual package names.
  3. The tool does not currently perform diffing recursively. Ie. creating the report on my stdlibs it does not generate a diff for package regexp/syntax. Consider searching using something like filepath.WalkDir.
  4. On the report, please make the lines which are == greyed out (putting css opacity: 0.75 probably works)

misc/stdlib_diff/diffstatus.go Outdated Show resolved Hide resolved
misc/stdlib_diff/algorithm.go Outdated Show resolved Hide resolved
misc/stdlib_diff/filediff.go Outdated Show resolved Hide resolved
misc/stdlib_diff/filediff.go Outdated Show resolved Hide resolved
misc/stdlib_diff/algorithm.go Outdated Show resolved Hide resolved
package main

import (
"slices"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seeing as you are importing slices, please add a go.mod file in this directory putting the go version at go 1.21

misc/stdlib_diff/myers.go Outdated Show resolved Hide resolved
misc/stdlib_diff/myers.go Outdated Show resolved Hide resolved
misc/stdlib_diff/report.go Outdated Show resolved Hide resolved
Comment on lines +158 to +169
f, err := os.Open(dirPath)
if err != nil {
return []string{}, nil
}

defer func() {
if err := f.Close(); err != nil {
fmt.Fprintln(os.Stderr, "can't close "+dirPath)
}
}()

filesInfo, err := f.Readdir(0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer os.ReadDir

@thehowl
Copy link
Member

thehowl commented Jan 8, 2024

@FloRichardAloeCorp I see some comments are still pending (including what I added in the top-level comment). Would you like me to do a second round of review or are you still making changes?

Thanks!

@FloRichardAloeCorp
Copy link
Author

@thehowl I still have work to do, I was waiting for your answers. I ping you back when I have finished.

@thehowl thehowl self-assigned this May 8, 2024
@zivkovicmilos
Copy link
Member

@thehowl what's the status on this PR?

@thehowl
Copy link
Member

thehowl commented Jun 24, 2024

@zivkovicmilos I'll get around to finishing what Florian started, eventually 🫠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Status: In Review
Development

Successfully merging this pull request may close these issues.

Tool for comparing source code of standard libaries between Go and Gno
3 participants