Page MenuHomePhabricator

Alexander-Shukaev (Alexander Shukaev)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 11 2015, 1:00 PM (354 w, 5 d)

Recent Activity

Sep 19 2017

Alexander-Shukaev added a comment to D15465: [git-clang-format]: New option to perform formatting against staged changes only.

Mark, just wanted to check if the review is still somewhere on your radar.

Sep 19 2017, 8:39 AM

Aug 30 2017

Alexander-Shukaev added a comment to D15465: [git-clang-format]: New option to perform formatting against staged changes only.

Hi @lodato, thanks mate.

Aug 30 2017, 7:05 AM
Alexander-Shukaev added a comment to D15465: [git-clang-format]: New option to perform formatting against staged changes only.

Did anybody have a chance to review it and/or try it out?

Aug 30 2017, 1:47 AM

Aug 26 2017

Alexander-Shukaev updated the diff for D15465: [git-clang-format]: New option to perform formatting against staged changes only.

Alright, so you got me excited about this task once again. Now, I've just rebased to the latest git-clang-format and it has changed a bit. Nevertheless, I've updated the previous patch accordingly and applied those changes which @lodato was proposing (except for getting rid of run_git_ls_files_and_save_to_tree as I'm still not sure whether this would be the same as git write-tree). Anyway, just tested the exact scenario posted by @lodato, and indeed with the current patch it works as he expects. Namely, there is no diff from unstaged tree interfering anymore. So far so good, but as I said in my previous post, there is another side of the medal. If you didn't get it, then read through it again and try, for example, the following amended @lodato's scenario:

Aug 26 2017, 3:48 PM

Aug 24 2017

Alexander-Shukaev added a comment to D15465: [git-clang-format]: New option to perform formatting against staged changes only.

Man, I have to admit it's really a shame that I didn't find time to work on this further but I'm truly too busy these days. However, I believe the primary point why I didn't have motivation to do this is because the flaw that was pointed out actually never bothered me in real life simply because I've never ever hit this case in production. I confess that I use this solution, namely the one linked on Stack Overflow (to my personal Bitbucket repository) every single day and we even introduced it in my company. Nobody ever complained so far. It's true that sometimes one would want to stage only some changes and commit them, while having other changes unstaged, but I don't remember when I was in that situation last time. If you really want to leave something out for another commit, then you can also stash those unstaged changes before you format/commit the staged ones. Furthermore, let me clarify why the proposal by @lodato might not even fit into the picture (i.e. there is no universal solution this problem as I see it until now). In particular, his example does not illustrate another side of the medal, namely let's say that the staged code was, in fact, badly formatted (not like in his example), and then you apply some code on top of it that is not yet staged (same like in his example). By "on top" I mean really like he shows it, that those changes overlap, that is if you'd stage them, they'd overwrite the previously staged ones (which in our imaginary example are badly formatted now). Now let's think what will happen if we follow his proposal. We'd apply formatting purely to the "staged" version of the file by piping it from index as a blob directly to formatter, so far so good. Or wait a minute, how would you actually format that file in place then? That is you already have unstaged and potentially conflicting changes with the ones you'd get as a result of formatting the staged ones but how to reconcile these two versions now? That is how do you put those formatted changes into unstaged state when you already have something completely different but also unstaged at the same line? Turns out that the answer is, you can't, without introducing explicit conflicts in the unstaged tree, which is even more confusing to my taste. Or would you just report the diff with an error message to the user leaving the act of applying those to them manually? You could, but then you give up on that cool feature of automatic formatting. To conclude, which approach you take, is all about pros and cons. On the daily basis and from productivity standpoint, I care more about doing my changes for the target commit, trying to commit, if something is wrong getting it automatically formatted in the unstaged tree, reviewing this unstaged diff quickly, staging it, and finally committing my work. That corner case with some irrelevant changes hanging in the unstaged tree and fooling formatter can rarely be a problem. And even then, I treat it more like an answer from a formatter: "Hey bro, you have some unstaged crap out there, can you please already decide whether you need it now or not, otherwise I can't do my job for you?!", in which case I will

Aug 24 2017, 1:50 PM

Feb 8 2016

Alexander-Shukaev retitled D15465: [git-clang-format]: New option to perform formatting against staged changes only from New 'git-clang-format' option to perform formatting against staged changes only to [git-clang-format]: New option to perform formatting against staged changes only.
Feb 8 2016, 9:14 AM

Jan 13 2016

Alexander-Shukaev added reviewers for D15465: [git-clang-format]: New option to perform formatting against staged changes only: DJKessler, klimek.
Jan 13 2016, 9:26 AM

Jan 5 2016

Alexander-Shukaev added a comment to D15465: [git-clang-format]: New option to perform formatting against staged changes only.

Can somebody, please, review the patch and provide feedback? It's been almost month since submission. I can add more reviewers if somebody points me to them.

Jan 5 2016, 6:37 AM

Dec 28 2015

Alexander-Shukaev added a comment to D15465: [git-clang-format]: New option to perform formatting against staged changes only.

The patch was updated accordingly. Please, give it one more shot. Thanks.

Dec 28 2015, 1:00 AM

Dec 22 2015

Alexander-Shukaev updated the diff for D15465: [git-clang-format]: New option to perform formatting against staged changes only.
  1. Fixed exception.
  2. Ensure to use the '--cached' option for diff only when the '--staged' option is supplied (just for logical correctness in general, but makes no real difference for 'git-clang-format' in particular).
Dec 22 2015, 2:48 PM
Alexander-Shukaev added a comment to D15465: [git-clang-format]: New option to perform formatting against staged changes only.

By no means I want to smell like an annoying bumper here, but just out of curiosity, is there any particular reason why the review on the patch is being delayed or reviewers are merely overloaded with other issues? Maybe I could add other reviewers if that's applicable?

Dec 22 2015, 7:00 AM

Dec 11 2015

Alexander-Shukaev updated D15465: [git-clang-format]: New option to perform formatting against staged changes only.
Dec 11 2015, 1:58 PM
Alexander-Shukaev retitled D15465: [git-clang-format]: New option to perform formatting against staged changes only from to New 'git-clang-format' option to perform formatting against staged changes only.
Dec 11 2015, 1:53 PM