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

Colored diff #1589

Merged
merged 4 commits into from
Oct 31, 2023
Merged

Colored diff #1589

merged 4 commits into from
Oct 31, 2023

Conversation

ksss
Copy link
Collaborator

@ksss ksss commented Oct 31, 2023

I added color by default to the diff format of the rbs diff command.

# t.rbs
class Foo
  def a: () -> void
  def c: () -> void
end
# tt.rbs
class Foo
  def a: () -> void
  def b: () -> void
  def c: () -> void
end
image image

Refactoring

Rename from RBS::Diff::CLI to RBS::CLI::Diff

Previously, we had a namespace for the Diff feature, but there were two more general CLI classes, making it confusing.
We've changed the name and placed the class for the Diff feature under the CLI class, renaming it to CLI::Diff.

Rename from RBS::Collection::ColoredIO to RBS::CLI::ColoredIO

We wanted to use ColoredIO within the CLI layer, but using Collection::ColoredIO at the CLI layer felt odd.
Moving ColoredIO to the CLI layer still felt natural within the Collection layer, so we made the shift.

Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

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

👍

@soutaro soutaro added this pull request to the merge queue Oct 31, 2023
@soutaro soutaro added this to the RBS 3.3 milestone Oct 31, 2023
Merged via the queue into ruby:master with commit a77932c Oct 31, 2023
23 checks passed
@soutaro soutaro added the Released PRs already included in the released version label Oct 31, 2023
@ksss ksss deleted the colored-diff branch October 31, 2023 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

2 participants