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

[WIP] refactoring blame walker #717

Closed
wants to merge 36 commits into from
Closed

[WIP] refactoring blame walker #717

wants to merge 36 commits into from

Conversation

randy3k
Copy link
Collaborator

@randy3k randy3k commented Jul 22, 2017

depends on #709 , if that is merge, this will be rebased. # 709 has merged.

It is a refactoring of the GsBlameCommand, specifically it brings these new features.

  1. syntax highlight

  2. line forwarding (the cursor will follow via history instead of going to a random place)

  3. use [m] to switch between detect within file, detect within commit and detect within all commits instead of showing a panel to ask every time. The origin popup is also modified to show less text.

  4. the options of the BlameActionCommand are renamed. Actually, still not sure the best to name them.

untitled


The maintainers of this repo require that all pull request submitters agree and adhere to the following:

  • I have read and agree to the contribution guidelines.
    (required)
  • All related documentation has been updated to reflect the changes made. (required)
  • My commit messages are cleaned up and ready to merge. (required)

The maintainers of this repository require you to select the semantic version type that
the changes in this pull request represent. Please select one of the following:

  • major
  • minor
  • patch
  • documentation only

@maintainerd
Copy link

maintainerd bot commented Jul 22, 2017

maintainerd logging is enabled for this repository.

All actions related to rules and their enforcement will be logged here as a permanent record.


Click to view log...

  • 2017-07-22T16:08:24.572Z:0a361f5: The pull request was created
  • 2017-07-22T16:08:36.289Z:0a361f5: @randy3k selected patch as the semantic version.
  • 2017-07-22T16:08:37.084Z:0a361f5: @randy3k checked My commit messages are cleaned up and ready to merge. _(required)_.
  • 2017-07-22T16:08:37.473Z:0a361f5: @randy3k checked All related documentation has been updated to reflect the changes made. _(required)_.
  • 2017-07-22T16:08:38.564Z:0a361f5: @randy3k checked I have read and agree to the [contribution guidelines](https://github.com/divmain/GitSavvy/blob/master/CONTRIBUTING.md)..
  • 2017-07-26T05:20:39.110Z:3ec4b43: @randy3k unchecked My commit messages are cleaned up and ready to merge. _(required)_.

@randy3k randy3k changed the title Randy3k/blame syntax highlight of blame walker Jul 22, 2017
@randy3k randy3k force-pushed the randy3k/blame branch 4 times, most recently from 9d58ea2 to 25617ff Compare July 23, 2017 03:52
@randy3k
Copy link
Collaborator Author

randy3k commented Jul 23, 2017

I tried it on a large repo of 30k commits and a relatively large file (3k lines). These are significant delay (~3s-7s) from the quick panel to the highlighting. I did some profiling, 50% of time was spent in scope_name. Another 50% of time was spent in the actual git blame command.

@randy3k randy3k force-pushed the randy3k/blame branch 13 times, most recently from ec05579 to ab028df Compare July 25, 2017 15:29
@randy3k randy3k changed the base branch from master to develope July 25, 2017 22:47
@randy3k randy3k added the next label Jul 27, 2017
@randy3k randy3k changed the title syntax highlight of blame walker [WIP] syntax highlight of blame walker Jul 27, 2017
@randy3k randy3k changed the title [WIP] syntax highlight of blame walker [WIP] refactoring blame walker Jul 27, 2017
@randy3k randy3k force-pushed the randy3k/blame branch 2 times, most recently from 7fbfd65 to c1b0970 Compare August 4, 2017 03:13
asfaltboy and others added 26 commits September 2, 2017 15:08
Internal: maintainerd - don't enforce empty line
Internal: add ISSUE_TEMPLATE.md and PULL_REQUEST_TEMPLATE.md
This change also refactors response validation for reuse
Feature: GitHub create fork command
Enhancement: select the branch name while creating a new branch
use `last_page_empty_message` instead of `empty_message` if the last page is empty
also refactor `show_paginated_panel` in a way discussed earlier
Feature: git stash in command panel
Before, we used a base ref that was specified in settings, otherwise defaulting to hardcoded "master".

Now, we find the nearest common ancestor branch (after some filtering) and use that as our default base, still allowing users to override it in settings.
- use .get_active_remote_branch().name_with_remote for clarity
- be consistant with string formatting in log calls
- limit to max revision count in `git rev-list` call
- rename max_iterations to max_revisions for clarity
This avoids wrongly using commit message parts that include strings
in square brackets as branches.

For example before this fix, the following commit would return WIP
as branch name: `[develo] [WIP] Fix: some bug`
- As suggested by @divmain the method was moved into a separate
  mixin for reuse, as well as, splitted into smaller parts

- Use `git branch --merged` instead of `--no-merged. This fixes a
  bug where the most obvious branches are not returned since no-merged
  means that only branches that are "unreachable" from given node
  are returned, and we actually want the opposite.
Feature: better selection of base ref branch
@randy3k
Copy link
Collaborator Author

randy3k commented Sep 11, 2017

It is hard to merge this. I am going to split this PR.

@randy3k randy3k closed this Sep 11, 2017
@randy3k randy3k mentioned this pull request Sep 11, 2017
7 tasks
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

Successfully merging this pull request may close these issues.

None yet

4 participants