{"thread":{"id":"8242","subject":"Re: [QGit PATCH] Remove most ASSERT warnings in Git::setStatus","startedAt":"2007-05-20T12:53:05Z","lastAt":"2007-05-20T16:06:28Z","messageCount":5,"participants":["Marco Costalba","Michael"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"42709","messageId":"e5bfff550705200553q757c334el7aa5aed393052616@mail.gmail.com","threadId":"8242","inReplyTo":"200705201401.35991.barra_cuda@katamail.com","subject":"Re: [QGit PATCH] Remove most ASSERT warnings in Git::setStatus","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T12:53:05Z","receivedAt":"2007-05-20T12:53:05Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Michael wrote:\n> Signed-off-by: Michele Ballabio \n> ---\n>\n> ...is this correct?\n>\n> src/git_startup.cpp | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/src/git_startup.cpp b/src/git_startup.cpp\n> index a99edba..17312f9 100644\n> --- a/src/git_startup.cpp\n> +++ b/src/git_startup.cpp\n> @@ -329,7 +329,7 @@ void Git::parseDiffFormatLine(RevFile& rf, SCRef line, int parNum) {\n>\n> // TODO rename/copy is not supported for combined merges\n> appendFileName(rf, line.section('\\t', -1));\n> - setStatus(rf, line.section(' ', 6, 6).left(1));\n> + setStatus(rf, line.section('\\t', -2, -2).right(1));\n> rf.mergeParent.append(parNum);\n> } else { // faster parsing in normal case\n>\n> --\n> 1.5.1.2\n>\n\nWhere do you see the ASSERT? could you link me to the test repository\nwhen you see that warnings?\n\nI would like to understand better what causes the warnings.\n\nThanks\nMarco\n"},{"id":"42714","messageId":"200705201608.06486.barra_cuda@katamail.com","threadId":"8242","inReplyTo":"e5bfff550705200553q757c334el7aa5aed393052616@mail.gmail.com","subject":"Re: [QGit PATCH] Remove most ASSERT warnings in Git::setStatus","fromName":"Michael","fromEmail":"barra_cuda@katamail.com","sentAt":"2007-05-20T14:08:06Z","receivedAt":"2007-05-20T14:08:06Z","isPatch":true,"sender":{"key":"barra_cuda@katamail.com","avatar":"https://avatars.githubusercontent.com/u/16371673?v=4"},"body":"(sorry, dropped git ml)\n\n\"Marco Costalba\" :\n> On 5/20/07, Michael wrote:\n> > Signed-off-by: Michele Ballabio \n> > ---\n> >\n> > ...is this correct?\n> >\n> > src/git_startup.cpp | 2 +-\n> > 1 files changed, 1 insertions(+), 1 deletions(-)\n> >\n> > diff --git a/src/git_startup.cpp b/src/git_startup.cpp\n> > index a99edba..17312f9 100644\n> > --- a/src/git_startup.cpp\n> > +++ b/src/git_startup.cpp\n> > @@ -329,7 +329,7 @@ void Git::parseDiffFormatLine(RevFile& rf, SCRef line, int parNum) {\n> >\n> > // TODO rename/copy is not supported for combined merges\n> > appendFileName(rf, line.section('\\t', -1));\n> > - setStatus(rf, line.section(' ', 6, 6).left(1));\n> > + setStatus(rf, line.section('\\t', -2, -2).right(1));\n> > rf.mergeParent.append(parNum);\n> > } else { // faster parsing in normal case\n> >\n> > --\n> > 1.5.1.2\n> >\n> \n> Where do you see the ASSERT? could you link me to the test repository\n> when you see that warnings?\n> \n> I would like to understand better what causes the warnings.\n\nOn the git repo:\n\nrm .git/qgit_cache.dat\nqgit origin/pu\nthen use arrows to go down to the first octopus merge, ie:\n\n Merge branches 'jc/blame' and 'jc/diff' into pu\n\nqgit prints:\n\nASSERT in Git::setStatus, unknown status <1>. 'MODIFIED' will be used instead.\nASSERT in Git::setStatus, unknown status . 'MODIFIED' will be used instead.\n\non stderr.\n\nThat's because the lines considered in Git::setStatus are:\n\n:::100644 100644 100644 100644 35864ed3c4afe01680bd5123fc28c35f5cf328e6 29243c6e8b49958ddcb08df0eb4223b14fd3e19f 16a5b9ac49c756492a7fd91fa49b84a3aee1f6b2 77ca8dcdfbcd6667b7f511306347c0d245ee4e2b MMM Makefile\n:::100644 100644 100644 100644 0e6439b0ddaf317a6288ab4dd40ae8b9a41e9884 4204bc168c11fc7f8764e7d92e5935d2dc30c3bd cbab8ebecb4f3f13856b3be21409074dbcd3edda d1ff7f50a31d9647c26c1e2293957a1d719c9373 MMM cache.h\n\n...which obviously do not have the right information (\"M\") in\n\n line.section(' ', 6, 6).left(1)\n\nbut in\n\n line.section('\\t', -2, -2).right(1)\n\nI thought this would trigger even with .git/qgit_cache.dat,\nbut I was wrong. (Anyway, I have a patch that adds an option\nto make .git/qgit_cache.dat optional, but I don't think it\ncould be that useful. What do you think?)\n\nIf it still doesn't trigger for you, maybe I should post you\nmy qgitrc...\n"},{"id":"42715","messageId":"e5bfff550705200723i7b4e21ebi6a2c51d66659b388@mail.gmail.com","threadId":"8242","inReplyTo":"200705201558.53546.barra_cuda@katamail.com","subject":"Re: [QGit PATCH] Remove most ASSERT warnings in Git::setStatus","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T14:23:29Z","receivedAt":"2007-05-20T14:23:29Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Michael wrote:\n> \"Marco Costalba\" :\n> > On 5/20/07, Michael wrote:\n> > > Signed-off-by: Michele Ballabio \n> > > ---\n> > >\n> > > ...is this correct?\n> > >\n> > > src/git_startup.cpp | 2 +-\n> > > 1 files changed, 1 insertions(+), 1 deletions(-)\n> > >\n> > > diff --git a/src/git_startup.cpp b/src/git_startup.cpp\n> > > index a99edba..17312f9 100644\n> > > --- a/src/git_startup.cpp\n> > > +++ b/src/git_startup.cpp\n> > > @@ -329,7 +329,7 @@ void Git::parseDiffFormatLine(RevFile& rf, SCRef line, int parNum) {\n> > >\n> > > // TODO rename/copy is not supported for combined merges\n> > > appendFileName(rf, line.section('\\t', -1));\n> > > - setStatus(rf, line.section(' ', 6, 6).left(1));\n> > > + setStatus(rf, line.section('\\t', -2, -2).right(1));\n> > > rf.mergeParent.append(parNum);\n> > > } else { // faster parsing in normal case\n> > >\n\n>From diff-format.txt we have that an output line is formatted this way:\n\n------------------------------------------------\nin-place edit :100644 100644 bcd1234... 0123456... M file0\ncopy-edit :100644 100644 abcd123... 1234567... C68 file1 file2\nrename-edit :100644 100644 abcd123... 1234567... R86 file1 file3\ncreate :000000 100644 0000000... 1234567... A file4\ndelete :100644 000000 1234567... 0000000... D file5\nunmerged :000000 000000 0000000... 0000000... U file6\n------------------------------------------------\n\nThat is, from the left to the right:\n\n. a colon.\n. mode for \"src\"; 000000 if creation or unmerged.\n. a space.\n. mode for \"dst\"; 000000 if deletion or unmerged.\n. a space.\n. sha1 for \"src\"; 0\\{40\\} if creation or unmerged.\n. a space.\n. sha1 for \"dst\"; 0\\{40\\} if creation, unmerged or \"look at work tree\".\n. a space.\n. status, followed by optional \"score\" number.\n. a tab or a NUL when '-z' option is used.\n. path for \"src\"\n. a tab or a NUL when '-z' option is used; only exists for C or R.\n. path for \"dst\"; only exists for C or R.\n. an LF or a NUL when '-z' option is used, to terminate the record.\n\nSo I rather would prefer something like\n\n line.section('\\t', 0, 0).section(' ', -1, -1).left(1)\n\nbecause we could have more then one file separated by a tab, so\n\n line.section('\\t', -2, -2).right(1)\n\nit seems to me a little bit fragile. What do you think?\n\nAlso I don't understand why you consider the right most (right(1)),\ninstead of the left most character as the status.\n\n\nThanks\nMarco\n"},{"id":"42716","messageId":"200705201641.20305.barra_cuda@katamail.com","threadId":"8242","inReplyTo":"e5bfff550705200723i7b4e21ebi6a2c51d66659b388@mail.gmail.com","subject":"Re: [QGit PATCH] Remove most ASSERT warnings in Git::setStatus","fromName":"Michael","fromEmail":"barra_cuda@katamail.com","sentAt":"2007-05-20T14:41:20Z","receivedAt":"2007-05-20T14:41:20Z","isPatch":true,"sender":{"key":"barra_cuda@katamail.com","avatar":"https://avatars.githubusercontent.com/u/16371673?v=4"},"body":"\"Marco Costalba\" :\n> So I rather would prefer something like\n> \n>   line.section('\\t', 0, 0).section(' ', -1, -1).left(1)\n> \n> because we could have more then one file separated by a tab, so\n> \n>  line.section('\\t', -2, -2).right(1)\n> \n> it seems to me a little bit fragile. What do you think?\n\nYou're right.\n\n> Also I don't understand why you consider the right most (right(1)),\n> instead of the left most character as the status.\n\nOnly because it was simpler AND because I didn't know it was wrong.\n"},{"id":"42724","messageId":"e5bfff550705200906x5464dc6bnc31954f5d1dcddd@mail.gmail.com","threadId":"8242","inReplyTo":"200705201641.20305.barra_cuda@katamail.com","subject":"Re: [QGit PATCH] Remove most ASSERT warnings in Git::setStatus","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-05-20T16:06:28Z","receivedAt":"2007-05-20T16:06:28Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 5/20/07, Michael wrote:\n> \"Marco Costalba\" :\n> > So I rather would prefer something like\n> >\n> > line.section('\\t', 0, 0).section(' ', -1, -1).left(1)\n> >\n> > because we could have more then one file separated by a tab, so\n> >\n> > line.section('\\t', -2, -2).right(1)\n> >\n> > it seems to me a little bit fragile. What do you think?\n>\n> You're right.\n>\n> > Also I don't understand why you consider the right most (right(1)),\n> > instead of the left most character as the status.\n>\n> Only because it was simpler AND because I didn't know it was wrong.\n>\n\nPatch pushed to public repo both for qgit and qgit4.\n\n\nGrazie per il report\nMarco\n"}]}