Thread: [cedet-semantic] semantic-analyze-proto-impl-toggle "could not find a suitable implementation"
Brought to you by:
zappo
Menu
▾
▴
cedet-semantic
|
From: Andreas H. <ah...@gm...> - 2010-09-14 07:23:27
|
I'm new to cedet and am trying to use it to make editing of C++ code more efficient. For a start I'm interested in the navigation features to jump from a header file to the implementation of a function in a cpp file and vice versa. I've mainly played with semantic-analyze-proto-impl-toggle and semantic-ia-fast-jump so far. To jump from the header to the cpp file, semantic-analyze-proto-impl-toggle works for the majority of cases, but for some it reports "could not find a suitable implementation", although there is one. Interestingly, sometimes when I switch to the buffer with the cpp file manually afterward, the cursor is actually right on the line it should have jumped to. So it seems to "almost work". Is there something I can do to improve the success-rate and make it switch buffers by itself? What kind of success-rate can I expect anyway? I've read the cedet website, "a gentle intro to cedet", some semantic info pages and looked through the mailing list archive and am under the impression this should just work, but I can't figure out what I need to tweak. Thanks, Andreas emacs 22.2.1 with cedet 1.0 on Windows XP, run from a MSYS/MinGW shell |
|
From: Andreas H. <ah...@gm...> - 2010-09-16 13:52:40
Attachments:
emacs-cedet-settings.txt
exif.hpp
|
Hello, To be more specific, attached are the results of running semantic-analyze-proto-impl-toggle on all the functions of a random C++ header file of the project I'm working on. I'm attaching the remainder of that file after removing all comments, #includes and 42 functions which work fine, with each function labeled with either "wrong jump" (13), if semantic-analyze-proto-impl-toggle jumped to the wrong function in the cpp file or "no impl" (also 13) if it could not find a suitable implementation. (The cases where it jumped to the wrong implementation are all for overloaded functions; it appears that it jumps to one particular implementation for all of them.) I'm still mainly wondering whether I'm doing something wrong on my side, so I'm also attaching my settings. The code is from this project: http://www.exiv2.org/exiv2-0.20.tar.gz, the original file is src/exif.hpp. Andreas On Tue, Sep 14, 2010 at 15:23, Andreas Huggel <ah...@gm...> wrote: > I'm new to cedet and am trying to use it to make editing of C++ code > more efficient. For a start I'm interested in the navigation features > to jump from a header file to the implementation of a function in a > cpp file and vice versa. I've mainly played with > semantic-analyze-proto-impl-toggle and semantic-ia-fast-jump so far. > > To jump from the header to the cpp file, > semantic-analyze-proto-impl-toggle works for the majority of cases, > but for some it reports "could not find a suitable implementation", > although there is one. Interestingly, sometimes when I switch to the > buffer with the cpp file manually afterward, the cursor is actually > right on the line it should have jumped to. So it seems to "almost > work". > > Is there something I can do to improve the success-rate and make it > switch buffers by itself? What kind of success-rate can I expect > anyway? I've read the cedet website, "a gentle intro to cedet", some > semantic info pages and looked through the mailing list archive and am > under the impression this should just work, but I can't figure out > what I need to tweak. > > Thanks, > Andreas > > emacs 22.2.1 with cedet 1.0 on Windows XP, run from a MSYS/MinGW shell > |
|
From: David E. <de...@ra...> - 2010-09-17 12:55:04
|
Andreas Huggel writes: > C++ header file of the project I'm working on. I'm attaching the > remainder of that file after removing all comments, #includes and 42 > functions which work fine, with each function labeled with either > "wrong jump" (13), if semantic-analyze-proto-impl-toggle jumped to the > wrong function in the cpp file or "no impl" (also 13) if it could not > find a suitable implementation. (The cases where it jumped to the > wrong implementation are all for overloaded functions; it appears that > it jumps to one particular implementation for all of them.) > explicit Exifdatum(const ExifKey& key, const Value* pValue =0); // no impl It's a bug: Semantic is mistaking 'explicit' for a type. Must be fixed in c.by. > Exifdatum(const Exifdatum& rhs); // no impl This one actually works for me. > virtual ~Exifdatum(); This is due to the :typemodifiers thingy for which Eric posted a patch recently > Exifdatum& operator=(const Exifdatum& rhs); // wrong jump > Exifdatum& operator=(const uint16_t& value); // wrong jump > Exifdatum& operator=(const uint32_t& value); // wrong jump > Exifdatum& operator=(const URational& value); // wrong jump > Exifdatum& operator=(const int16_t& value); // wrong jump > Exifdatum& operator=(const int32_t& value); // wrong jump > Exifdatum& operator=(const Rational& value); // wrong jump > Exifdatum& operator=(const std::string& value); // wrong jump Yes, overloading is a problem. I think currently Semantic doesn't look at the arguments when comparing the tags. [...] > #ifdef EXV_UNICODE_PATH > long writeFile(const std::wstring& wpath) const; // no impl > #endif Not parsed because of the #ifdef. You'll probably have to manually set the symbol to non-nil. > iterator erase(iterator pos); // no impl Here I guess this is simply because of fully-qualified vs short, meaning it expects a 'iterator' but gets 'ExifData::iterator'. This should be easy to fix. Regards, David |
|
From: David E. <de...@ra...> - 2010-09-19 15:23:00
|
Andreas Huggel writes:
> On Sun, Sep 19, 2010 at 21:07, David Engster <de...@ra...> wrote:
>> their type that matters. Hence I think that tag-similar-p should check
>> the arguments; I attached a patch.
>
> This patch fixes all of the reported "wrong jumps" for overloaded
> functions except for this case in class Exifdatum:
>
> std::string toString() const; // wrong jump
> std::string toString(long n) const;
>
> That's already much better, thanks!
Well, I'd bet the 'explicit' thing is also not working? :-)
This can be fixed by adding the token
%token EXPLICIT "explicit"
%put EXPLICIT summary "Forbids implicit type conversion: explicit <constructor>"
in c.by.
This already avoids that the keyword is identified as a type. To be
extra sure, we could also add something like
| EXPLICIT
()
for example in builtin-type (although it's not a type, but where else to
put this?).
Regards,
David
|
|
From: Andreas H. <ah...@gm...> - 2010-09-19 16:38:13
|
On Sun, Sep 19, 2010 at 23:22, David Engster wrote:
> Well, I'd bet the 'explicit' thing is also not working? :-)
>
> This can be fixed by adding the token
>
> %token EXPLICIT "explicit"
> %put EXPLICIT summary "Forbids implicit type conversion: explicit <constructor>"
>
> in c.by.
explicit Exifdatum(const ExifKey& key, const Value* pValue =0);
Yes, with these two lines that works too. Thanks!
Andreas
|
|
From: Eric M. L. <er...@si...> - 2010-09-24 02:15:11
|
On 09/19/2010 11:22 AM, David Engster wrote: > Andreas Huggel writes: >> On Sun, Sep 19, 2010 at 21:07, David Engster<de...@ra...> wrote: >>> their type that matters. Hence I think that tag-similar-p should check >>> the arguments; I attached a patch. >> >> This patch fixes all of the reported "wrong jumps" for overloaded >> functions except for this case in class Exifdatum: >> >> std::string toString() const; // wrong jump >> std::string toString(long n) const; >> >> That's already much better, thanks! > > Well, I'd bet the 'explicit' thing is also not working? :-) > > This can be fixed by adding the token > > %token EXPLICIT "explicit" > %put EXPLICIT summary "Forbids implicit type conversion: explicit<constructor>" > > in c.by. > > This already avoids that the keyword is identified as a type. To be > extra sure, we could also add something like > > | EXPLICIT > () > > for example in builtin-type (although it's not a type, but where else to > put this?). This could be added to the DECLMOD rule, which includes things like EXTERN and STATIC. I know it is just for constructors, but c.by can't find constructors since it doesn't keep a state table to check against along the way. Eric |
|
From: David E. <de...@ra...> - 2010-09-19 15:40:34
|
Eric M. Ludlam writes:
> I discovered that this AM too. There are a bunch of funny cases with
> arguments though. In the tests there are examples where the arg name
> exists in one place but not another, for example, so I have some new
> stuff to make that work.
I see that you allow one name to be blank. But if I remember correctly,
in overloaded functions the name doesn't matter at all and can be
different. This is why in my patch I override the name with this hack
(setq tmp1 (append '("name") (cdr (nth counter args))))
(setq tmp2 (append '("name") (cdr (nth counter v))))
so that the recursive call to tag-similar-p will effectively ignore
it. Adding something like :name to ignore-attributes is nicer, for sure.
> The function you mention is only used in one unit test. I'm tempted
> to remove it in favor of just one function doing the right thing
> unless someone has a reason to keep the two distinct.
I see no reason to keep it.
Regards,
David
|
|
From: David E. <de...@ra...> - 2010-09-24 14:25:59
|
Eric M. Ludlam writes:
> Since this is my first batch of bzr checkins that I think have made, it
> would be great if folks could give it a try. I also checked in past
> diffs I had posted here.
Maybe I'm mistaken, but from the logs it seems to me that you directly
pushed to trunk from within a task branch, meaning you did
bzr push bzr+ssh://CEDET-URL/code/trunk
while still in the 'similarp' branch. This is not a good idea with
bzr. Maybe this is OK as long as there are no merges from trunk
involved, but in any case, I'd avoid it. In Bazaar, revision numbers are
always specific to a certain branch. If you push a task branch upstream,
this will make upstream a mirror of your task branch, possibly with new
revision numbers. This can lead to a strange history display for other
developers which are again merging your changes from trunk (because they
are in fact merging the history of your task branch).
Bzr encourages a 'merge to local trunk branch and push' workflow: When
you have finished some feature on a separate task branch, usually
involving several smaller commits, you merge those changes into the
trunk and then push. This is described in Lluis' docs in the last
paragraph titled "Merging into the mainline". You would then choose a
commit messages which shortly summarizes the changes you did in that
task branch; in the normal history view with 'bzr log', you'd only see
that summary, not the single commits you did in that branch.
By doing so, you get a hierarchical history, which can be seen fully by
using "bzr log -n0" instead of the default "bzr log", which uses "-n1".
Just try those two commands on the Emacs repo to get an impression. The
latter will only show you the merge messages ("flat view"), while the
former will also display the commits contained in those merges (those
are indented).
If you think this stuff is overkill, I'd say just work directly on the
trunk; this pretty much gives you a workflow like an 'offline CVS', in
which you explicitly have to push changes upstream.
-David
|
|
From: David E. <de...@ra...> - 2010-10-04 17:50:47
Attachments:
similarp-patch.diff
|
Andreas Huggel writes:
>> Since this is my first batch of bzr checkins that I think have made, it
>> would be great if folks could give it a try. I also checked in past
>> diffs I had posted here.
>
> I've re-run my black-box tests from above with bzr revision 7986 and
> emacs 22.3.1 on Mac OS X 10.6.4
> Out of the 26 issues reported earlier, 20 are fixed. Great! The
> remaining cases are:
>
> class EXIV2API Exifdatum : public Metadatum {
> public:
> explicit Exifdatum(const ExifKey& key, const Value* pValue
> =0); // no impl
> std::string toString() const; // wrong jump
> std::string toString(long n) const; // ok!
> };
>
> class EXIV2API ExifData {
> public:
> iterator erase(iterator pos); // no impl
> iterator erase(iterator beg, iterator end); // no impl
> iterator findKey(const ExifKey& key); // no impl
> const_iterator findKey(const ExifKey& key) const; // no impl
> };
Could you please try if the attached patch fixes these remaining issues?
Eric, are you OK with those changes?
-David
|
|
From: Andreas H. <ah...@gm...> - 2010-10-05 15:33:54
|
On Tue, Oct 5, 2010 at 01:50, David Engster wrote: > > Could you please try if the attached patch fixes these remaining issues? > Ran all the test-cases again (bzr r7986 + similarp-patch.diff). The only issue left is the explicit constructor explicit Exifdatum(const ExifKey& key, const Value* pValue =0); // no impl Thanks! Andreas |
|
From: Eric M. L. <er...@si...> - 2010-09-18 00:28:55
|
Thanks for looking at this... On 09/17/2010 08:54 AM, David Engster wrote: >> Exifdatum& operator=(const Exifdatum& rhs); // wrong jump >> > Exifdatum& operator=(const uint16_t& value); // wrong jump >> > Exifdatum& operator=(const uint32_t& value); // wrong jump >> > Exifdatum& operator=(const URational& value); // wrong jump >> > Exifdatum& operator=(const int16_t& value); // wrong jump >> > Exifdatum& operator=(const int32_t& value); // wrong jump >> > Exifdatum& operator=(const Rational& value); // wrong jump >> > Exifdatum& operator=(const std::string& value); // wrong jump > Yes, overloading is a problem. I think currently Semantic doesn't look > at the arguments when comparing the tags. Semantic does look at arguments in tag-similar-p when trying to resolve cases like this. I don't think I ever tested on operators though. > >> > iterator erase(iterator pos); // no impl > Here I guess this is simply because of fully-qualified vs short, meaning > it expects a 'iterator' but gets 'ExifData::iterator'. This should be > easy to fix. For the argument, this would be tricky. iterator will need to be resolved to a fully qualified namespace before jumping to the source, or the reverse, all possible hits will need to be expanded. If "erase" is in a namespace or class, then that is should already be handled. This is where I might be tempted to add a hack if something good was found, but nothing was "similar", then just picking one is perhaps better than nothing. Eric |
|
From: Andreas H. <ah...@gm...> - 2010-09-18 13:10:02
|
> On 09/17/2010 08:54 AM, David Engster wrote: >>> Exifdatum& operator=(const Exifdatum& rhs); // wrong jump >>> > Exifdatum& operator=(const uint16_t& value); // wrong jump >>> > Exifdatum& operator=(const uint32_t& value); // wrong jump >>> > Exifdatum& operator=(const URational& value); // wrong jump >>> > Exifdatum& operator=(const int16_t& value); // wrong jump >>> > Exifdatum& operator=(const int32_t& value); // wrong jump >>> > Exifdatum& operator=(const Rational& value); // wrong jump >>> > Exifdatum& operator=(const std::string& value); // wrong jump >> Yes, overloading is a problem. I think currently Semantic doesn't look >> at the arguments when comparing the tags. > > > Semantic does look at arguments in tag-similar-p when trying to resolve > cases like this. I don't think I ever tested on operators though. This affects not only overloaded operators in the sample file. All overloaded functions behave the same. Andreas |
|
From: Andreas H. <ah...@gm...> - 2010-09-18 13:00:12
|
Thanks for your feedback. I'm now running cedet from CVS (and emacs
23.2.1 on Linux).
>> Exifdatum(const Exifdatum& rhs); // no impl
>
> This one actually works for me.
Yes, this works now.
>> virtual ~Exifdatum();
>
> This is due to the :typemodifiers thingy for which Eric posted a patch recently
That still doesn't work ("no impl"). Is that patch in CVS?
>> #ifdef EXV_UNICODE_PATH
>> long writeFile(const std::wstring& wpath) const; // no impl
>> #endif
>
> Not parsed because of the #ifdef. You'll probably have to manually set
> the symbol to non-nil.
I've set the symbol, now it jumps to the wrong implementation,
probably because this is also an overloaded function.
>> iterator erase(iterator pos); // no impl
>
> Here I guess this is simply because of fully-qualified vs short, meaning
> it expects a 'iterator' but gets 'ExifData::iterator'. This should be
> easy to fix.
If I change the signature to ExifData::iterator
erase(ExifData::iterator pos) it works indeed. But C++ doesn't require
this.
Andreas
|
|
From: David E. <de...@ra...> - 2010-09-19 06:18:43
|
Andreas Huggel writes:
>>> virtual ~Exifdatum();
>>
>> This is due to the :typemodifiers thingy for which Eric posted a patch recently
>
> That still doesn't work ("no impl"). Is that patch in CVS?
No. CEDET has switched to Bazaar, and I think Eric hasn't pushed yet. :-)
>>> #ifdef EXV_UNICODE_PATH
>>> long writeFile(const std::wstring& wpath) const; // no impl
>>> #endif
>>
>> Not parsed because of the #ifdef. You'll probably have to manually set
>> the symbol to non-nil.
>
> I've set the symbol, now it jumps to the wrong implementation,
> probably because this is also an overloaded function.
I'll look into the Overloading problem.
>>> iterator erase(iterator pos); // no impl
>>
>> Here I guess this is simply because of fully-qualified vs short, meaning
>> it expects a 'iterator' but gets 'ExifData::iterator'. This should be
>> easy to fix.
>
> If I change the signature to ExifData::iterator
> erase(ExifData::iterator pos) it works indeed. But C++ doesn't require
> this.
With 'easy to fix' I meant Semantic. Your code indeed needs no fixing. :-)
Seems like I was a bit premature on the 'easy' part though, but I'll
look into it.
-David
|
|
From: Eric M. L. <er...@si...> - 2010-09-19 14:22:03
Attachments:
semantic-tag.el.diff
|
On 09/19/2010 09:07 AM, David Engster wrote: > Eric M. Ludlam writes: >> On 09/17/2010 08:54 AM, David Engster wrote: >>>> Exifdatum& operator=(const Exifdatum& rhs); // wrong jump >>>>> Exifdatum& operator=(const uint16_t& value); // wrong jump >>>>> Exifdatum& operator=(const uint32_t& value); // wrong jump >>>>> Exifdatum& operator=(const URational& value); // wrong jump >>>>> Exifdatum& operator=(const int16_t& value); // wrong jump >>>>> Exifdatum& operator=(const int32_t& value); // wrong jump >>>>> Exifdatum& operator=(const Rational& value); // wrong jump >>>>> Exifdatum& operator=(const std::string& value); // wrong jump >>> Yes, overloading is a problem. I think currently Semantic doesn't look >>> at the arguments when comparing the tags. >> >> >> Semantic does look at arguments in tag-similar-p when trying to resolve >> cases like this. I don't think I ever tested on operators though. > > No, it doesn't descent into sublists of tags: > > ;; Don't test sublists of tags > ((and (listp v) (semantic-tag-p (car v))) > nil) > > There exists `semantic-tag-similar-with-subtags-p', and we could call > that one for tags with :arguments property, but it also checks the names > of the arguments, and in the case of overloaded functions, it is only > their type that matters. Hence I think that tag-similar-p should check > the arguments; I attached a patch. I discovered that this AM too. There are a bunch of funny cases with arguments though. In the tests there are examples where the arg name exists in one place but not another, for example, so I have some new stuff to make that work. The function you mention is only used in one unit test. I'm tempted to remove it in favor of just one function doing the right thing unless someone has a reason to keep the two distinct. >>>>> iterator erase(iterator pos); // no impl >>> Here I guess this is simply because of fully-qualified vs short, meaning >>> it expects a 'iterator' but gets 'ExifData::iterator'. This should be >>> easy to fix. >> >> For the argument, this would be tricky. iterator will need to be >> resolved to a fully qualified namespace before jumping to the source, >> or the reverse, all possible hits will need to be expanded. > > Yes, this would be the correct way to do it. My first thought was to > simply use a hack and ignore namespaces - just split the name and only > compare the last element. Using semantic-split-name would be a quick solution. Sadly, I can't seem to figure out bzr enough to check my changes in in the time I have available this AM so I'll attach a diff instead. Eric |
|
From: Andreas H. <ah...@gm...> - 2010-09-19 14:40:59
|
On Sun, Sep 19, 2010 at 21:07, David Engster <de...@ra...> wrote:
> their type that matters. Hence I think that tag-similar-p should check
> the arguments; I attached a patch.
This patch fixes all of the reported "wrong jumps" for overloaded
functions except for this case in class Exifdatum:
std::string toString() const; // wrong jump
std::string toString(long n) const;
That's already much better, thanks!
Andreas
|
|
From: David E. <de...@ra...> - 2010-09-19 20:26:24
|
Andreas Huggel writes: > On Sun, Sep 19, 2010 at 21:07, David Engster <de...@ra...> wrote: >> their type that matters. Hence I think that tag-similar-p should check >> the arguments; I attached a patch. > > This patch fixes all of the reported "wrong jumps" for overloaded > functions except for this case in class Exifdatum: > > std::string toString() const; // wrong jump > std::string toString(long n) const; Those are nice test cases you got there. ;-) This one indeed fell through. While tag-similar-p checks if both tags have the same number of attributes, it also counts the attributes which should be ignored. As it so happens, the first toString doesn't have :arguments, but it does have a :prototype-flag, so the number of attributes with the (wrong) second implementation is the same. It's not hard to hack a little workaround, but I guess Eric wants to rewrite that function anyway. :-) (Actually, to really catch everything, one would have to check if both tags really have the same set of attributes). -David |
|
From: Eric M. L. <er...@si...> - 2010-09-20 11:41:48
|
On 09/19/2010 11:40 AM, David Engster wrote:
> Eric M. Ludlam writes:
>> I discovered that this AM too. There are a bunch of funny cases with
>> arguments though. In the tests there are examples where the arg name
>> exists in one place but not another, for example, so I have some new
>> stuff to make that work.
>
> I see that you allow one name to be blank. But if I remember correctly,
> in overloaded functions the name doesn't matter at all and can be
> different. This is why in my patch I override the name with this hack
>
> (setq tmp1 (append '("name") (cdr (nth counter args))))
> (setq tmp2 (append '("name") (cdr (nth counter v))))
>
> so that the recursive call to tag-similar-p will effectively ignore
> it. Adding something like :name to ignore-attributes is nicer, for sure.
>
I hadn't checked your patch earlier because I had written mine before
you posted.
The info about names being irrelevant is interesting. It indicates that
how the comparison is made should be mode specific, and that the
comparison system probably is too complex for semantic-tag.el.
semantic-tag.el has traditionally been only very simple code.
semantic-tag-ls.el is where I've put language specific, yet relatively
simple stuff.
I'm usually a bit wary of modifying tags. The code you have above looks
safe. Sometimes tags from the main database can get modified
unexpectedly, so you could use `semantic-tag-clone' to do the same thing.
David, if you'd like to keep poking at this that would be great. I
still need to figure out bzr.
Eric
|
|
From: David E. <de...@ra...> - 2010-09-20 21:01:16
|
Eric M. Ludlam writes: > The info about names being irrelevant is interesting. It indicates that > how the comparison is made should be mode specific, and that the > comparison system probably is too complex for semantic-tag.el. > semantic-tag.el has traditionally been only very simple code. > semantic-tag-ls.el is where I've put language specific, yet relatively > simple stuff. I think it would make sense to make semantic-tag-similar-p a mode-local function, with the current implementation being the default, ignoring sublists of tags. I can then create a specific override for C(++). > I'm usually a bit wary of modifying tags. The code you have above looks > safe. Sometimes tags from the main database can get modified > unexpectedly, so you could use `semantic-tag-clone' to do the same thing. Yes, one has to be careful not to modify tags in-place, I already experienced issues with that. But since I don't modify but only append the cdr, I don't see how the tag could be modified, but using tag-clone surely is less hacky, so I see your point. David |
|
From: Eric M. L. <er...@si...> - 2010-09-24 02:08:34
|
On 09/19/2010 11:40 AM, David Engster wrote:
> Eric M. Ludlam writes:
>> I discovered that this AM too. There are a bunch of funny cases with
>> arguments though. In the tests there are examples where the arg name
>> exists in one place but not another, for example, so I have some new
>> stuff to make that work.
>
> I see that you allow one name to be blank. But if I remember correctly,
> in overloaded functions the name doesn't matter at all and can be
> different. This is why in my patch I override the name with this hack
>
> (setq tmp1 (append '("name") (cdr (nth counter args))))
> (setq tmp2 (append '("name") (cdr (nth counter v))))
>
> so that the recursive call to tag-similar-p will effectively ignore
> it. Adding something like :name to ignore-attributes is nicer, for sure.
>
I have finally figured out the basics of bzr, and have checked in
changes for tag-similar-p. As discussed, it has mode overrides, and C
mode. In there, it will completely ignore names for arguments as you
specify above, and the tests validate it. The code is now in
semantic-tag-ls.el too.
Since this is my first batch of bzr checkins that I think have made, it
would be great if folks could give it a try. I also checked in past
diffs I had posted here.
Thanks
Eric
|
|
From: Eric M. L. <er...@si...> - 2010-09-24 22:06:09
|
On 09/24/2010 10:25 AM, David Engster wrote:
> Eric M. Ludlam writes:
>> Since this is my first batch of bzr checkins that I think have made, it
>> would be great if folks could give it a try. I also checked in past
>> diffs I had posted here.
>
> Maybe I'm mistaken, but from the logs it seems to me that you directly
> pushed to trunk from within a task branch, meaning you did
>
> bzr push bzr+ssh://CEDET-URL/code/trunk
>
> while still in the 'similarp' branch. This is not a good idea with
> bzr. Maybe this is OK as long as there are no merges from trunk
> involved, but in any case, I'd avoid it. In Bazaar, revision numbers are
> always specific to a certain branch. If you push a task branch upstream,
> this will make upstream a mirror of your task branch, possibly with new
> revision numbers. This can lead to a strange history display for other
> developers which are again merging your changes from trunk (because they
> are in fact merging the history of your task branch).
>
> Bzr encourages a 'merge to local trunk branch and push' workflow: When
> you have finished some feature on a separate task branch, usually
> involving several smaller commits, you merge those changes into the
> trunk and then push. This is described in Lluis' docs in the last
> paragraph titled "Merging into the mainline". You would then choose a
> commit messages which shortly summarizes the changes you did in that
> task branch; in the normal history view with 'bzr log', you'd only see
> that summary, not the single commits you did in that branch.
>
> By doing so, you get a hierarchical history, which can be seen fully by
> using "bzr log -n0" instead of the default "bzr log", which uses "-n1".
> Just try those two commands on the Emacs repo to get an impression. The
> latter will only show you the merge messages ("flat view"), while the
> former will also display the commits contained in those merges (those
> are indented).
>
> If you think this stuff is overkill, I'd say just work directly on the
> trunk; this pretty much gives you a workflow like an 'offline CVS', in
> which you explicitly have to push changes upstream.
Hi David,
I figured some of that out after the fact. I had used the checkout
branch technique first just to get some little stuff in. That meant I
head 'trunk' as a checkout. When I went to merge similarp into trunk it
failed (as far as I can remember) because it wasn't the right kind of
trunk. Reading through I saw how push worked, tried it, and it did stuff.
I suspect I would use a checkout of trunk for most things, since I do
lots of small patches, but I like the idea of having branches for bigger
tasks. I then am stuck with deleting trunk each time I do an operation?
The nice thing about my CVS workflow was that I just pointed Emacs at
it, and I was good to go. All the branching means I have to restart
Emacs all the time. If I'm deleting trunk a lot that would be a challenge.
I'm sure there's a good workflow to use. I just need to break away from
my old one.
Eric
|
|
From: David E. <de...@ra...> - 2010-09-25 09:46:13
|
Eric M. Ludlam writes: > I figured some of that out after the fact. I had used the checkout > branch technique first just to get some little stuff in. That meant I > head 'trunk' as a checkout. When I went to merge similarp into trunk it > failed (as far as I can remember) because it wasn't the right kind of > trunk. Reading through I saw how push worked, tried it, and it did stuff. > > I suspect I would use a checkout of trunk for most things, since I do > lots of small patches, but I like the idea of having branches for bigger > tasks. I then am stuck with deleting trunk each time I do an operation? No. You can do 'bzr unbind' to convert a checkout into a regular branch (and 'bind' does vice versa). But you already see that using checkouts with task branches make stuff actually more complicated. I don't see a reason to use checkouts at all, unless you're developing something entirely on your own (and are always online). Instead, for (relatively) quick fixes, just work on the trunk branch, do your commits, and do 'bzr push <URL>' when finished (you only have to specify the URL the first time; it will be remembered afterwards). This has two main advantages over checkouts: You can do commits while offline, and the commits can be much more granular, since you don't have to worry about breaking the build upstream - you only push when you've finished something. You probably remember that my continuous integration testing for CEDET sometimes reported build errors because you were right in the middle of fixing something when the build-test ran. > The nice thing about my CVS workflow was that I just pointed Emacs at > it, and I was good to go. All the branching means I have to restart > Emacs all the time. If I'm deleting trunk a lot that would be a challenge. You should never have to delete the trunk branch. Restarting Emacs with a changed load-path would only be necessary to test a task branch. As you already wrote, task branches are really useful for bigger stuff. For example, I'm currently using a task branch for the F90 support, regularly merging from trunk to detect conflicts early on. This makes a later merge into the trunk much easier. CEDET is different from projects like Emacs in that ~99% of the commits are done by you. Hence, stuff like 'merging from trunk' is not as crucial to you, since your trunk branch will almost always be up-to-date, and you can just avoid creating a conflict with one of your task branches in the first place. Actually, it's people like me which profit much more from a DVCS than you. I can work for weeks on some feature, regularly merge from trunk to detect conflicts early on, and then merge or push a separate branch for you to review. So, I'd also like to thank Lluis for the terrific work he has done so far with the bzr transition! -David |
|
From: Andreas H. <ah...@gm...> - 2010-09-27 14:51:42
|
> Since this is my first batch of bzr checkins that I think have made, it
> would be great if folks could give it a try. I also checked in past
> diffs I had posted here.
I've re-run my black-box tests from above with bzr revision 7986 and
emacs 22.3.1 on Mac OS X 10.6.4
Out of the 26 issues reported earlier, 20 are fixed. Great! The
remaining cases are:
class EXIV2API Exifdatum : public Metadatum {
public:
explicit Exifdatum(const ExifKey& key, const Value* pValue
=0); // no impl
std::string toString() const; // wrong jump
std::string toString(long n) const; // ok!
};
class EXIV2API ExifData {
public:
iterator erase(iterator pos); // no impl
iterator erase(iterator beg, iterator end); // no impl
iterator findKey(const ExifKey& key); // no impl
const_iterator findKey(const ExifKey& key) const; // no impl
};
Andreas
|
|
From: Eric M. L. <er...@si...> - 2010-10-06 00:12:46
|
On 10/04/2010 01:50 PM, David Engster wrote:
> Andreas Huggel writes:
> Eric, are you OK with those changes?
Hi David,
Thanks for looking into this. I like your change. I have only one comment:
> +(define-mode-local-override semantic--tag-similar-types-p c-mode
(tag1 tag2)
> + "For c-mode, if only one of the types is fully qualfied, we
> +split its name and compare the last element."
In this case my mail says there is a qualified typo.
But aside from that, after splitting the names, should it compare the
last N strings, where N is min( num_tag1_strs, num_tag2_strs ) instead?
That way if you have:
foo::bar::someclass myfcn();
noodle::poodle::someclass myfcn() { }
it won't get confused?
Regardless, this a a good compromise over doing a full analysis on the
types which would be a bit slow.
Eric
|
|
From: David E. <de...@ra...> - 2010-10-16 17:22:10
|
Eric M. Ludlam writes:
> On 10/04/2010 01:50 PM, David Engster wrote:
> > +(define-mode-local-override semantic--tag-similar-types-p c-mode
> (tag1 tag2)
> > + "For c-mode, if only one of the types is fully qualfied, we
> > +split its name and compare the last element."
>
> In this case my mail says there is a qualified typo.
:-)
> But aside from that, after splitting the names, should it compare the
> last N strings, where N is min( num_tag1_strs, num_tag2_strs ) instead?
> That way if you have:
>
> foo::bar::someclass myfcn();
>
> noodle::poodle::someclass myfcn() { }
>
> it won't get confused?
The code did only compare types in case one of them is a string, i.e.,
not qualified at all. But you are right that comparing the last N
strings is better, since one of the types could just be 'shorter' (half
qualified? whatever...), meaning it wouldn't detect that those two
namespace foo {
bar::someclass myfcn();
}
foo::bar::someclass myfcn();
are in fact the same.
I've rewritten the similar-type function to do what you've suggested,
and I've finally merged that branch into trunk (use 'bzr log -n0' to see
the single commits).
Andreas, the current trunk should be able to deal with all the problems
you've encountered (or so I hope...).
-David
|
Thanks for helping keep SourceForge clean.
X