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

Add new package "Git Blame Status Bar" #7151

Closed
wants to merge 2 commits into from

Conversation

rodrigobdz
Copy link

Shows Git blame author for the currently selected line.

Shows Git blame author for the currently selected line.
Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: ERROR

Repo link: Git Blame Status Bar
Results help

Packages added:
  - Git Blame Status Bar

Processing package "Git Blame Status Bar"
  - ERROR: No valid semver tags found at https://github.com/rodrigobdz/subl-gitblame-statusbar/tags for the package "Git Blame Status Bar".

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: SUCCESS

Repo link: Git Blame Status Bar

Packages added:
  - Git Blame Status Bar

Processing package "Git Blame Status Bar"
  - All checks passed

@Thom1729
Copy link
Collaborator

I see that this is partly derived from the Git blame package. Would it make sense to add this feature to that package rather than creating a new package?

@rodrigobdz
Copy link
Author

@Thom1729 I thought about it but decided to publish a new package instead to offer a minimalistic solution to a common problem.
Git Blame sure offers more functionality but IMO it has a different goal than Git Blame Status Bar.

@rodrigobdz
Copy link
Author

Git Blame Status Bar has an approach comparable to the one of the VSCode Extension Active File In Status Bar.
Minimal solution to a simple problem.

@Thom1729
Copy link
Collaborator

  • You should explicitly handle the case where the file is not in a git repo.
  • Generally, ignoring errors is not ideal.
  • You should throttle multiple activations to avoid performance problems.

What I would do myself is separate the code into a TextCommand that does all of the real work and a ViewEventListener that invokes that command. I'd also add some sort of timeout to prevent the listener from activating the command too frequently.

@Thom1729
Copy link
Collaborator

@rodrigobdz ping.

@Thom1729 Thom1729 added the stale The pull request needs to be updated but has not been within the recent past (2 weeks) label Aug 31, 2018
@rodrigobdz
Copy link
Author

@Thom1729 I haven't forgotten this open PR. I haven't got around to it yet.

@FichteFoll FichteFoll added timeout A pull request needed changes but was not updated in time (2 weeks after becoming stale) and removed stale The pull request needs to be updated but has not been within the recent past (2 weeks) labels Dec 31, 2018
@FichteFoll FichteFoll closed this Dec 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
timeout A pull request needed changes but was not updated in time (2 weeks after becoming stale)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants