Two feature ideas for diffs

Two feature ideas for diffs

[Tom]

I was reading this description of PyCharm 

   http://nicoddemus.github.io/articles/pycharm/

and there was two features described which could be useful in
emacs too.

The first is diff indicators in the sidebar. If the file is under
version control and it is edited then a mark appears beside the
changed lines and if it is clicked then you can see what's
changed in the line or in that hunk, you don't have to do a full
file diff which may contain many other changes unrelated to the
current line. You can also rollback that particular change,
etc. Is there something like this for emacs? If not it could be a
useful addition.

The other interesting feature is editable diffs. So, for example,
before checking in one usually does a diff to see what was
changed in the file. If you notice a typo then you go back to the
file, fix it, do a diff again to check the changes, etc. PyCharm
lets you edit the diff instead, so if you made a typo then you
can fix it in the diff buffer and it is applied to the original
file. It would be a nice convenience feature.

Re: Two feature ideas for diffs

[nhs]

> The first is diff indicators in the sidebar. If the file is under
> version control and it is edited then a mark appears beside the
> changed lines and if it is clicked then you can see what's
> changed in the line or in that hunk, you don't have to do a full
> file diff which may contain many other changes unrelated to the
> current line. You can also rollback that particular change,
> etc. Is there something like this for emacs? If not it could be a
> useful addition.

There are git-gutter and git-gutter+. As the name suggests, they only 
work with git, but do what you describe.

     Nicolas

Re: Two feature ideas for diffs

[Tom]

nhs <nhs <at> openmailbox.org> writes:

> 
> There are git-gutter and git-gutter+. As the name suggests, they only 
> work with git, but do what you describe.
> 

Nice, but it's a pity it's written as git-specific. Git should
only be one of the backends and the frontend should call the
appropriate backend after checking what VC the file is under.

Re: Two feature ideas for diffs

[Dmitry Gutov]

Tom <adatgyujto@gmail.com> writes:

> nhs <nhs <at> openmailbox.org> writes:
>> There are git-gutter and git-gutter+. As the name suggests, they only 
>> work with git, but do what you describe.
>
> Nice, but it's a pity it's written as git-specific. Git should
> only be one of the backends and the frontend should call the
> appropriate backend after checking what VC the file is under.

Check out https://github.com/dgutov/diff-hl

Re: Two feature ideas for diffs

[Tom]

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> Check out https://github.com/dgutov/diff-hl
> 


Thanks. I haven't yet tried it, but it looks promising.

Re: Two feature ideas for diffs

[Stefan Monnier]

> Check out https://github.com/dgutov/diff-hl

Would be good to add it to GNU ELPA,


        Stefan

Re: Two feature ideas for diffs

[Dmitry Gutov]

On 07.11.2013 21:09, Stefan Monnier wrote:
>> Check out https://github.com/dgutov/diff-hl
>
> Would be good to add it to GNU ELPA,

Sure. I was thinking it might use a rename, though ("Git Gutter" has a 
much better ring to it), and this would be a good time.

Any suggestions? Or do you think the name is fine?

Re: Two feature ideas for diffs

[Stefan Monnier]

> Any suggestions? Or do you think the name is fine?

Bikeshedding is tempting, but I'll pass,


        Stefan

Re: Two feature ideas for diffs

[Michael Heerdegen]

Hi all,

> Check out https://github.com/dgutov/diff-hl

That's nice.

In addition, I would like to see the vc state in the mode-line.  I see
"Git-master" in the mode-line and a mouse echo telling the state ("Up to
date"; "Locally modified" etc.).  But I want to see the state from the
indicator - preferably by using faces.  I tried to use this in
`mode-line-format' (using my own faces):

(vc-mode (:eval (propertize vc-mode 'face
                  (pcase (vc-state buffer-file-truename)
                    (`up-to-date 'mode-line-important)
                    (`edited                      nil)
                    (_             'mode-line-warning)))))

But that doesn't work.  The state is never refreshed, I need to restart
Emacs.  (vc-state buffer-file-truename) seems to return the same value
for the entire Emacs session.  I'm not familiar with the VC code - how
can it be done?  And shouldn't vc-state return an up-to-date value (bug)?


Thanks,

Michael.

 

Re: Two feature ideas for diffs

[Óscar Fuentes]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hi all,
>
>> Check out https://github.com/dgutov/diff-hl
>
> That's nice.
>
> In addition, I would like to see the vc state in the mode-line.  I see
> "Git-master" in the mode-line and a mouse echo telling the state ("Up to
> date"; "Locally modified" etc.).  But I want to see the state from the
> indicator

The char between "Git" and "master" indicates the VC state. `-' means up
to date, `:' means "modified", etc

> - preferably by using faces.

I implemented that on my personal branch of Emacs. IIRC I submitted it
for inclusion, but possibly it went lost on the mailing list.

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 1d67dee..657980c 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -204,12 +204,12 @@ matching the resulting Git log output, and KEYWORDS is a list of
     (?M 'edited)
     (?A 'added)
     (?D 'removed)
-    (?U 'edited)     ;; FIXME
+    (?U 'conflict)
     (?T 'edited)))   ;; FIXME
 
 (defun vc-git-state (file)
   "Git-specific version of `vc-state'."
-  ;; FIXME: This can't set 'ignored or 'conflict yet
+  ;; FIXME: This can't set 'ignored yet.
   ;; The 'ignored state could be detected with `git ls-files -i -o
   ;; --exclude-standard` It also can't set 'needs-update or
   ;; 'needs-merge. The rough equivalent would be that upstream branch
@@ -781,6 +781,7 @@ If LIMIT is non-nil, show no more than this many entries."
                     ,(format "--pretty=tformat:%s"
 			     (car vc-git-root-log-format))
 		    "--abbrev-commit"))
+		'("--follow")
 		(when limit (list "-n" (format "%s" limit)))
 		(when start-revision (list start-revision))
 		'("--")))))))
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index c47bc4c..3140157 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -34,6 +34,93 @@
 
 ;; Customization Variables (the rest is in vc.el)
 
+(defvar vc-ignore-vc-files nil)
+(make-obsolete-variable 'vc-ignore-vc-files
+                        "set `vc-handled-backends' to nil to disable VC."
+			"21.1")
+
+(defvar vc-master-templates ())
+(make-obsolete-variable 'vc-master-templates
+ "to define master templates for a given BACKEND, use
+vc-BACKEND-master-templates.  To enable or disable VC for a given
+BACKEND, use `vc-handled-backends'."
+ "21.1")
+
+(defvar vc-header-alist ())
+(make-obsolete-variable 'vc-header-alist 'vc-BACKEND-header "21.1")
+
+(defface vc-up-to-date-state
+  '((((class color) (background light))
+     :foreground "blue1")
+    (((class color) (background dark))
+     :foreground "white"))
+  "Face for VC modeline state when the file is up to date."
+  :version "24.1"
+  :group 'vc)
+
+(defface vc-needs-update-state
+  '((((class color) (background light))
+     :foreground "blue1")
+    (((class color) (background dark))
+     :foreground "white"))
+  "Face for VC modeline state when the file needs update."
+  :version "24.1"
+  :group 'vc)
+
+(defface vc-locked-state
+  '((((class color) (background light))
+     :foreground "blue1" :weight bold)
+    (((class color) (background dark))
+     :foreground "white" :weight bold))
+  "Face for VC modeline state when the file locked."
+  :version "24.1"
+  :group 'vc)
+
+(defface vc-locally-added-state
+  '((((class color) (background light))
+     :foreground "red" :weight bold)
+    (((class color) (background dark))
+     :foreground "red" :weight bold))
+  "Face for VC modeline state when the file is locally added."
+  :version "24.1"
+  :group 'vc)
+
+(defface vc-conflict-state
+  '((((class color) (background light))
+     :foreground "black" :background "red")
+    (((class color) (background dark))
+     :foreground "white" :background "red"))
+  "Face for VC modeline state when the file contains merge conflicts."
+  :version "24.1"
+  :group 'vc)
+
+(defface vc-removed-state
+  '((((class color) (background light))
+     :foreground "black" :background "red" :weight bold)
+    (((class color) (background dark))
+     :foreground "white" :background "red" :weight bold))
+  "Face for VC modeline state when the file was removed from the VC system."
+  :version "24.1"
+  :group 'vc)
+
+(defface vc-missing-state
+  '((((class color) (background light))
+     :foreground "black" :background "red" :weight bold)
+    (((class color) (background dark))
+     :foreground "white" :background "red" :weight bold))
+  "Face for VC modeline state when the file is missing from the file system."
+  :version "24.1"
+  :group 'vc)
+
+(defface vc-edited-state
+  '((((class color) (background light))
+     :foreground "red")
+    (((class color) (background dark))
+     :foreground "red"))
+  "Face for VC modeline state when the file is up to date."
+  :version "24.1"
+  :group 'vc)
+
 (defcustom vc-ignore-dir-regexp
   ;; Stop SMB, automounter, AFS, and DFS host lookups.
   locate-dominating-stop-dir-regexp
@@ -788,33 +875,42 @@ This function assumes that the file is registered."
   (let* ((backend-name (symbol-name backend))
 	 (state   (vc-state file backend))
 	 (state-echo nil)
+	 (face nil)
 	 (rev     (vc-working-revision file backend)))
     (propertize
      (cond ((or (eq state 'up-to-date)
 		(eq state 'needs-update))
 	    (setq state-echo "Up to date file")
+	    (setq face 'vc-up-to-date-state)
 	    (concat backend-name "-" rev))
 	   ((stringp state)
 	    (setq state-echo (concat "File locked by" state))
+	    (setq face 'vc-locked-state)
 	    (concat backend-name ":" state ":" rev))
            ((eq state 'added)
             (setq state-echo "Locally added file")
+	    (setq face 'vc-locally-added-state)
             (concat backend-name "@" rev))
            ((eq state 'conflict)
             (setq state-echo "File contains conflicts after the last merge")
+	    (setq face 'vc-conflict-state)
             (concat backend-name "!" rev))
            ((eq state 'removed)
             (setq state-echo "File removed from the VC system")
+	    (setq face 'vc-removed-state)
             (concat backend-name "!" rev))
            ((eq state 'missing)
             (setq state-echo "File tracked by the VC system, but missing from the file system")
+	    (setq face 'vc-missing-state)
             (concat backend-name "?" rev))
 	   (t
 	    ;; Not just for the 'edited state, but also a fallback
 	    ;; for all other states.  Think about different symbols
 	    ;; for 'needs-update and 'needs-merge.
 	    (setq state-echo "Locally modified file")
+	    (setq face 'vc-edited-state)
 	    (concat backend-name ":" rev)))
+     'face face
      'help-echo (concat state-echo " under the " backend-name
 			" version control system"))))
 

Re: Two feature ideas for diffs

[Michael Heerdegen]

Óscar Fuentes <ofv@wanadoo.es> writes:

> > In addition, I would like to see the vc state in the mode-line.  I see
> > "Git-master" in the mode-line and a mouse echo telling the state ("Up to
> > date"; "Locally modified" etc.).  But I want to see the state from the
> > indicator
>
> The char between "Git" and "master" indicates the VC state. `-' means up
> to date, `:' means "modified", etc

Uff, I would never have guessed that, not in a thousand years.

> > - preferably by using faces.
>
> I implemented that on my personal branch of Emacs. IIRC I submitted it
> for inclusion, but possibly it went lost on the mailing list.

Having something like that would be nice.  Dunno if coloring the
mode-line is acceptable...?

BTW, in the meanwhile, I found out that

  (vc-state (expand-file-name buffer-file-truename))

is always correct.  Seems `vc-state' needs an expanded file name.


Regards,

Michael.

Re: Two feature ideas for diffs

[Stefan Monnier]

> Uff, I would never have guessed that, not in a thousand years.

Don't need to guess.  It says so in the manual.


        Stefan

Re: Two feature ideas for diffs

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > Uff, I would never have guessed that, not in a thousand years.
>
> Don't need to guess.  It says so in the manual.

I've read it now.  Didn't think that Emacs VC has that many features,
not bad.  Sorry, but I thought the colon is just a ... colon (like in
"Term: char run" in M-x term).  It's IMHO not very intuitive.

Thanks,

Michael.

Re: Two feature ideas for diffs

[Stefan Monnier]

> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -204,12 +204,12 @@ matching the resulting Git log output, and KEYWORDS is a list of
>      (?M 'edited)
>      (?A 'added)
>      (?D 'removed)
> -    (?U 'edited)     ;; FIXME
> +    (?U 'conflict)
>      (?T 'edited)))   ;; FIXME
 
>  (defun vc-git-state (file)
>    "Git-specific version of `vc-state'."
> -  ;; FIXME: This can't set 'ignored or 'conflict yet
> +  ;; FIXME: This can't set 'ignored yet.
>    ;; The 'ignored state could be detected with `git ls-files -i -o
>    ;; --exclude-standard` It also can't set 'needs-update or
>    ;; 'needs-merge. The rough equivalent would be that upstream branch
> @@ -781,6 +781,7 @@ If LIMIT is non-nil, show no more than this many entries."
>                      ,(format "--pretty=tformat:%s"
>  			     (car vc-git-root-log-format))
>  		    "--abbrev-commit"))
> +		'("--follow")
>  		(when limit (list "-n" (format "%s" limit)))
>  		(when start-revision (list start-revision))
>  		'("--")))))))

Is it really all it takes to detect `conflict' state?


        Stefan

Re: Two feature ideas for diffs

[Óscar Fuentes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Is it really all it takes to detect `conflict' state?

Git conflict state detection worked when I implemented the feature many
moons ago, although it was not 100% reliable. It doesn't work anymore.
Maybe something was lost on one of the many merges.

Re: Two feature ideas for diffs

[Stefan Monnier]

>> Is it really all it takes to detect `conflict' state?
> Git conflict state detection worked when I implemented the feature many
> moons ago, although it was not 100% reliable. It doesn't work anymore.
> Maybe something was lost on one of the many merges.

Could you try and make it work again and submit/install the patch for that?


        Stefan

Git conflict VC state detection

[Óscar Fuentes]

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Is it really all it takes to detect `conflict' state?
>> Git conflict state detection worked when I implemented the feature many
>> moons ago, although it was not 100% reliable. It doesn't work anymore.
>> Maybe something was lost on one of the many merges.
>
> Could you try and make it work again and submit/install the patch for that?

I've revised the initial commit implementing conflict state detection on
my private branch and the problem with it was that it didn't detect
edited state on a staged file, i.e. a file containing staged (but
uncommitted) changes would show as up to date.

So I pursued a more precise method. The problem is that the git commands
that tells you that a file is unmerged doesn't tell that a file contains
changes if those are staged. The solution is to execute two git
commands: the first one informs about edited (but unstaged) files plus
unmerged files. If that command says that the file is up to date, a
second command is executed for detecting staged changes.

The problem with this approach is that on some platforms (i.e. Windows)
invoking git is slow. The delay is already noticeable when the modeline
is updated. Adding yet another call makes things somewhat worse.

I CC'ed Dmitry as he implemented the part that my patch touches. I'll
like to get the ok from him before installing the patch, if you (Stefan)
think that conflict detection is valuable enough to warrant an extra git
invocation.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-git.patch --]
[-- Type: text/x-diff, Size: 3070 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 1d67dee..cb500eb 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -204,15 +204,26 @@ matching the resulting Git log output, and KEYWORDS is a list of
     (?M 'edited)
     (?A 'added)
     (?D 'removed)
-    (?U 'edited)     ;; FIXME
+    (?U 'conflict)
     (?T 'edited)))   ;; FIXME
 
+(defun vc-git--state-letter (file index-p)
+  (let ((diff (vc-git--run-command-string
+               file
+	       "diff-index"
+	       ;; How to conditionally add --cached without this kludge?
+	       (if index-p "--cached" "-z")
+	       "--name-status" "--raw" "-z" "HEAD" "--")))
+    (and diff
+	 (string-match "^\\([ADMUT]\\)\0" diff)
+	 (match-string 1 diff))))
+
 (defun vc-git-state (file)
   "Git-specific version of `vc-state'."
-  ;; FIXME: This can't set 'ignored or 'conflict yet
+  ;; FIXME: This can't set 'ignored yet.
   ;; The 'ignored state could be detected with `git ls-files -i -o
-  ;; --exclude-standard` It also can't set 'needs-update or
-  ;; 'needs-merge. The rough equivalent would be that upstream branch
+  ;; --exclude-standard` It also can't set 'needs-update.
+  ;; The rough equivalent would be that upstream branch
   ;; for current branch is in fast-forward state i.e. current branch
   ;; is direct ancestor of corresponding upstream branch, and the file
   ;; was modified upstream.  But we can't check that without a network
@@ -220,21 +231,16 @@ matching the resulting Git log output, and KEYWORDS is a list of
   ;; This assumes that status is known to be not `unregistered' because
   ;; we've been successfully dispatched here from `vc-state', that
   ;; means `vc-git-registered' returned t earlier once.  Bug#11757
-  (let ((diff (vc-git--run-command-string
-               file "diff-index" "-p" "--raw" "-z" "HEAD" "--")))
-    (if (and diff
-             (string-match ":[0-7]\\{6\\} [0-7]\\{6\\} [0-9a-f]\\{40\\} [0-9a-f]\\{40\\} \\([ADMUT]\\)\0[^\0]+\0\\(.*\n.\\)?"
-                           diff))
-        (let ((diff-letter (match-string 1 diff)))
-          (if (not (match-beginning 2))
-              ;; Empty diff: file contents is the same as the HEAD
-              ;; revision, but timestamps are different (eg, file
-              ;; was "touch"ed).  Update timestamp in index:
-              (prog1 'up-to-date
-                (vc-git--call nil "add" "--refresh" "--"
-                              (file-relative-name file)))
-            (vc-git--state-code diff-letter)))
-      (if (vc-git--empty-db-p) 'added 'up-to-date))))
+  (let ((diff-letter (or (vc-git--state-letter file t)
+			 (vc-git--state-letter file nil))))
+    (if (not diff-letter)
+	;; Empty diff: file contents is the same as the HEAD
+	;; revision, but timestamps are different (eg, file
+	;; was "touch"ed).  Update timestamp in index:
+	(prog1 'up-to-date
+	  (vc-git--call nil "add" "--refresh" "--"
+			(file-relative-name file)))
+      (vc-git--state-code diff-letter))))
 
 (defun vc-git-working-revision (file)
   "Git-specific version of `vc-working-revision'."

Re: Git conflict VC state detection

[Dmitry Gutov]

On 10.11.2013 22:30, Óscar Fuentes wrote:
> I've revised the initial commit implementing conflict state detection on
> my private branch and the problem with it was that it didn't detect
> edited state on a staged file, i.e. a file containing staged (but
> uncommitted) changes would show as up to date.
>
> So I pursued a more precise method. The problem is that the git commands
> that tells you that a file is unmerged doesn't tell that a file contains
> changes if those are staged.

Are you sure? AFAICS, 'git diff' behaves this way ('git diff' shows only 
unstages changes, 'git diff --cached' only stages ones, but 'git diff 
HEAD' will include both), but the 'git diff-index' command we're using 
already includes HEAD as the reference, and even if I do a 'git add' on 
some edited file, while 'git diff' shows me nothing, 'git diff-index 
--raw HEAD' includes the file with status M.

I haven't tested it in the situation with a file being merged, though. 
Maybe that's different somehow.

> I CC'ed Dmitry as he implemented the part that my patch touches. I'll
> like to get the ok from him before installing the patch,

I did a relatively small change there, actually, it just unwrapped all 
the code in `vc-git-registered'. But FWIW, I don't like the idea of 
doing double the work (external process calls) to know about one more 
status, in principle.

Re: Git conflict VC state detection

[Óscar Fuentes]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 10.11.2013 22:30, Óscar Fuentes wrote:
>> I've revised the initial commit implementing conflict state detection on
>> my private branch and the problem with it was that it didn't detect
>> edited state on a staged file, i.e. a file containing staged (but
>> uncommitted) changes would show as up to date.
>>
>> So I pursued a more precise method. The problem is that the git commands
>> that tells you that a file is unmerged doesn't tell that a file contains
>> changes if those are staged.
>
> Are you sure? AFAICS, 'git diff' behaves this way ('git diff' shows
> only unstages changes, 'git diff --cached' only stages ones, but 'git
> diff HEAD' will include both), but the 'git diff-index' command we're
> using already includes HEAD as the reference, and even if I do a 'git
> add' on some edited file, while 'git diff' shows me nothing, 'git
> diff-index --raw HEAD' includes the file with status M.
>
> I haven't tested it in the situation with a file being merged, though.
> Maybe that's different somehow.

With the current method in vc-git-state, this is an example where
the file keyword.cpp is unmerged:

$ git diff-index HEAD -- keyword.cpp

(-p --raw options eluded, as they just add the patch to the output,
which is useless.)

:100644 100644 12b8b5a24d99dfec20c4b5283d91b15ed1a83b4e 0000000000000000000000000000000000000000 Mkeyword.cpp

At first, you can think that those series of zeroes mean something (the
documentation for git-diff-index says

``sha1 for "dst"; 0{40} if creation, unmerged or "look at work tree"

which is quite vague) so let's try with a plainly edited file:

$ git diff-index HEAD -- CMakeLists.txt

:100644 100644 8465aca146db66862498b9e2f2a8993e5d8dda69 0000000000000000000000000000000000000000 MCMakeLists.txt

Hence `git diff-index' can't tell the difference among an unmerged and
an edited file.

Now let's try again but with the `--cached' option:

$ git diff-index --cached -- keyword.cpp

:100644 000000 12b8b5a24d99dfec20c4b5283d91b15ed1a83b4e 0000000000000000000000000000000000000000 Ukeyword.cpp

There it is the `U'. But with the edited, unstaged file:

$ git diff-index --cached HEAD -- CMakeLists.txt 

<no output>

And then you need a plain "git diff-index HEAD -- CMakeLists.txt" for
knowing that the file is edited.

AFAIK there is no command that tells that a file is unmerged and also
tells that it is edited, no matter if its changes are staged or not.

>> I CC'ed Dmitry as he implemented the part that my patch touches. I'll
>> like to get the ok from him before installing the patch,
>
> I did a relatively small change there, actually, it just unwrapped all
> the code in `vc-git-registered'.

So you were wrongly "blamed" :-) Sorry about that.

> But FWIW, I don't like the idea of
> doing double the work (external process calls) to know about one more
> status, in principle.

I'm not myself happy with the solution. It all depends on how important
it is to know the `conflict' status. Maybe other features on Emacs VC
depends on it.

Re: Git conflict VC state detection

[Stefan Monnier]

> I'm not myself happy with the solution. It all depends on how important
> it is to know the `conflict' status. Maybe other features on Emacs VC
> depends on it.

The feature that depends on this information is to automatically enable
smerge-mode when there's a conflict and to automatically mark the
conflicts as resolved when there are no more diff3 markers (see how
it's done is vc-svn.el and vc-bzr.el).

So, an alternative to fixing vc-state is to add a vc-git-conflict-p called
from the find-file-hook.

Another useful feature along those lines is to add support for
vc-find-conflicted-file.


        Stefan

Re: Git conflict VC state detection

[Óscar Fuentes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> So, an alternative to fixing vc-state is to add a vc-git-conflict-p called
> from the find-file-hook.

IIUC what you propose does not reduce the number of git invocations
while visiting/reverting a file but disperses the code change over a
broader area.

Re: Git conflict VC state detection

[Stefan Monnier]

>> So, an alternative to fixing vc-state is to add a vc-git-conflict-p called
>> from the find-file-hook.
> IIUC what you propose does not reduce the number of git invocations
> while visiting/reverting a file

Indeed :-(

It can reduce it in a few other cases where vc-state is called without
visiting/reverting a file.  Admittedly, these are rare (mostly because
vc-dir doesn't use vc-state but some other function).

Maybe another option is to do:
if status is ambiguous and could be `conflict', look for diff3 markers,
and if found, run git a second time to confirm that the status is
`conflict'.  Of course, if we could get the `conflict' status in
a single git invocation that would be much better.


        Stefan

Re: Git conflict VC state detection

[Dmitry Gutov]

On 11.11.2013 00:19, Óscar Fuentes wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>> On 10.11.2013 22:30, Óscar Fuentes wrote:
>>> I've revised the initial commit implementing conflict state detection on
>>> my private branch and the problem with it was that it didn't detect
>>> edited state on a staged file, i.e. a file containing staged (but
>>> uncommitted) changes would show as up to date.
>>>
>>> So I pursued a more precise method. The problem is that the git commands
>>> that tells you that a file is unmerged doesn't tell that a file contains
>>> changes if those are staged.
>>
>> Are you sure? AFAICS, 'git diff' behaves this way ('git diff' shows
>> only unstages changes, 'git diff --cached' only stages ones, but 'git
>> diff HEAD' will include both), but the 'git diff-index' command we're
>> using already includes HEAD as the reference, and even if I do a 'git
>> add' on some edited file, while 'git diff' shows me nothing, 'git
>> diff-index --raw HEAD' includes the file with status M.
>>
>> I haven't tested it in the situation with a file being merged, though.
>> Maybe that's different somehow.
>
> With the current method in vc-git-state, this is an example where
> the file keyword.cpp is unmerged:
>
> $ git diff-index HEAD -- keyword.cpp
>
> (-p --raw options eluded, as they just add the patch to the output,
> which is useless.)
>
> :100644 100644 12b8b5a24d99dfec20c4b5283d91b15ed1a83b4e 0000000000000000000000000000000000000000 Mkeyword.cpp
>
> At first, you can think that those series of zeroes mean something (the
> documentation for git-diff-index says
>
> ``sha1 for "dst"; 0{40} if creation, unmerged or "look at work tree"
>
> which is quite vague) so let's try with a plainly edited file:
>
> $ git diff-index HEAD -- CMakeLists.txt
>
> :100644 100644 8465aca146db66862498b9e2f2a8993e5d8dda69 0000000000000000000000000000000000000000 MCMakeLists.txt
>
> Hence `git diff-index' can't tell the difference among an unmerged and
> an edited file.

I see.

> Now let's try again but with the `--cached' option:
>
> $ git diff-index --cached -- keyword.cpp
>
> :100644 000000 12b8b5a24d99dfec20c4b5283d91b15ed1a83b4e 0000000000000000000000000000000000000000 Ukeyword.cpp
>
> There it is the `U'. But with the edited, unstaged file:
>
> $ git diff-index --cached HEAD -- CMakeLists.txt
>
> <no output>
>
> And then you need a plain "git diff-index HEAD -- CMakeLists.txt" for
> knowing that the file is edited.
>
> AFAIK there is no command that tells that a file is unmerged and also
> tells that it is edited, no matter if its changes are staged or not.

Your example tells me that if 'git diff-index --cached' returns status 
'U', then plain 'git diff-index' has to return status 'M', doesn't it?

IOW, if you call the command without '--cached' first, you can avoid the 
second invocation if the status is not 'M'.

Or, better yet, also check for diff3 markers, like Stefan suggested.

>>> I CC'ed Dmitry as he implemented the part that my patch touches. I'll
>>> like to get the ok from him before installing the patch,
>>
>> I did a relatively small change there, actually, it just unwrapped all
>> the code in `vc-git-registered'.
>
> So you were wrongly "blamed" :-) Sorry about that.

Not a problem.

Re: Two feature ideas for diffs

[Dmitry Gutov]

Tom <adatgyujto@gmail.com> writes:

> The other interesting feature is editable diffs. So, for example,
> before checking in one usually does a diff to see what was
> changed in the file. If you notice a typo then you go back to the
> file, fix it, do a diff again to check the changes, etc. PyCharm
> lets you edit the diff instead, so if you made a typo then you
> can fix it in the diff buffer and it is applied to the original
> file. It would be a nice convenience feature.

I've been pointed to https://github.com/caldwell/commit-patch in the
past, but it depends on Perl, and I haven't gotten around to trying it.

Re: Two feature ideas for diffs

[Tom]

Dmitry Gutov <dgutov <at> yandex.ru> writes:
> 
> I've been pointed to https://github.com/caldwell/commit-patch in the
> past, but it depends on Perl, and I haven't gotten around to trying it.

The described feature does not seem that complicated if diff editing
supports only simple changes (e.g. no adding/deleting lines, only changing
existing lines in the diff buffer). It should be enough (if one wants to
do extensive modifications then he should go back to the file instead),
so relying on external tools seems an overkill. 

Re: Two feature ideas for diffs

[Ivan Andrus]

On Nov 7, 2013, at 3:11 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:

> Tom <adatgyujto@gmail.com> writes:
> 
>> The other interesting feature is editable diffs. So, for example,
>> before checking in one usually does a diff to see what was
>> changed in the file. If you notice a typo then you go back to the
>> file, fix it, do a diff again to check the changes, etc. PyCharm
>> lets you edit the diff instead, so if you made a typo then you
>> can fix it in the diff buffer and it is applied to the original
>> file. It would be a nice convenience feature.
> 
> I've been pointed to https://github.com/caldwell/commit-patch in the
> past, but it depends on Perl, and I haven't gotten around to trying it.

I can give commit-patch a hearty two thumbs up (I've used it mostly with mercurial and git).

-Ivan

Re: Two feature ideas for diffs

[Stefan Monnier]

> The other interesting feature is editable diffs.  So, for example,
> before checking in one usually does a diff to see what was
> changed in the file.  If you notice a typo then you go back to the
> file, fix it, do a diff again to check the changes, etc.  PyCharm
> lets you edit the diff instead, so if you made a typo then you
> can fix it in the diff buffer and it is applied to the original
> file.  It would be a nice convenience feature.

diff-mode has supported editing the diffs "for ever".
It does not propagate the changes back to the source file, tho.
Patches to do that are welcome,


        Stefan

Re: Two feature ideas for diffs

[Barry OReilly]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

> Nice, but it's a pity it's written as git-specific. Git should only
> be one of the backends and the frontend should call the appropriate
> backend after checking what VC the file is under.

It should operate on unified diffs and provide the customization for
how to obtain the unified diffs, eg 'git diff'.

Dmitry, can diff-hl be used to review patch files distributed as diff
files without relying on a VCS?

[-- Attachment #2: Type: text/html, Size: 493 bytes --]

Re: Two feature ideas for diffs

[Dmitry Gutov]

Barry OReilly <gundaetiapo@gmail.com> writes:

>> Nice, but it's a pity it's written as git-specific. Git should only
>> be one of the backends and the frontend should call the appropriate
>> backend after checking what VC the file is under.
>
> It should operate on unified diffs and provide the customization for
> how to obtain the unified diffs, eg 'git diff'.
>
> Dmitry, can diff-hl be used to review patch files distributed as diff
> files without relying on a VCS?

No, it's rather tied to the VC package.  But why would you want that?

The main benefit of diff-hl is to see the diff on the margin updated as
you edit and save the file.