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

Efficient hunking of multiple hunks #1118

Merged
merged 1 commit into from Jul 13, 2019
Merged

Conversation

kaste
Copy link
Collaborator

@kaste kaste commented Feb 25, 2019

Fixes #1105

Multiple hunks can be combined into a patch which then can applied at once.


def apply_diffs_for_pts(self, cursor_pts, reset):
in_cached_mode = self.view.settings().get("git_savvy.diff_view.in_cached_mode")
patches = unique(flatten(filter_(self.head_and_hunk_for_pt(pt) for pt in cursor_pts)))
Copy link
Member

Choose a reason for hiding this comment

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

Correct me if I'm wrong but I believe we can simply use a set (or set comprehension), as head_and_hunk_for_pt returns either None or substr (which is guaranteed to be a string). What do you say?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm, which parts of this line sould be replaceable? btw: head_and_hunk_for_pt -> Optional[Tuple[str, str]]

So we go:

			List[Optional[Tuple[str, str]]] 
- filter -> List[Tuple[str, str]]
- flat   -> List[str]

Copy link
Member

Choose a reason for hiding this comment

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

Yea tuples of strings are usable in set just fine. I'm suggesting:

patches = ''.join(set(flatten(filter_(self.head_and_hunk_for_pt(pt) for pt in cursor_pts))))

But now that I think of it, it might not be good to jumble up the order of the strings; at the very least the head and hunk should come together ? I think we can scrape my suggestion in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes for the outer unique, we must keep everything ordered.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Basically

HEAD_A, HUNK_B, HEAD_A, HUNK_C, ...
->
HEAD_A, HUNK_B, HUNK_C

Copy link
Member

@asfaltboy asfaltboy left a comment

Choose a reason for hiding this comment

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

Nice, love the mypy stubs for sublime! Are these sourced from somewhere? And if so, can we mention it (link to it) in the comment?

@kaste
Copy link
Collaborator Author

kaste commented Jul 11, 2019

If you will I have the ©️ on the stubs. You start with stubgen (CLI tool that ships with mypy, just try it!), but then basically every Type everywhere is just Any 😒 but you don't have to write all the method names at least unless the tool fails for some class...

@kaste
Copy link
Collaborator Author

kaste commented Jul 11, 2019

I should put them on GitHub and link them to here and SublimeLinter etc of course. Not sure how to do that easily.

If the user selected multiple hunks, we merge them to a single patch and apply
it.
@asfaltboy
Copy link
Member

asfaltboy commented Jul 13, 2019

@kaste this only applies to standard diff, not inline-diff right? I'm not certain, but it feels to me like we should be allowing multi-line / multi-hunk staging in inline diff as well, maybe in a separate PR

Anyway, this works fine for me, so let's merge

@asfaltboy asfaltboy merged commit 349ef78 into timbrel:dev Jul 13, 2019
@kaste kaste deleted the efficient-hunking branch July 13, 2019 08:36
@kaste
Copy link
Collaborator Author

kaste commented Jul 13, 2019

Hooray

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

2 participants