Fixed the 5000 line wrap bug in CVS

[Entropy]

A C wrapper is nice, I don't have any objections.

[Florian]

- as_array()

 

Hmm, as char ** ?

 

Don't you prefer to use iterators (in C) for that ?

 

Álvaro

I didn't want to change that much in the current code, but I'll leave that decision to more experienced programmers.

[Schmurk]

Hum, I haven't looked at this part of the code yet but do we really need to use C++ to handle such a basic thing?

To be honest, I'm against mixing C/C++ code unless we don't have the choice like for the cal3d lib. And making wrappers between C/C++ doesn't simplify the code IMO.

But well, it's just my opinion...

[Learner]

Hum, I haven't looked at this part of the code yet but do we really need to use C++ to handle such a basic thing?

To be honest, I'm against mixing C/C++ code unless we don't have the choice like for the cal3d lib. And making wrappers between C/C++ doesn't simplify the code IMO.

But well, it's just my opinion...

Another solution could be an array of pointers so you have less data to move or a linked list, and then malloc/strdup and free the memory.

[alvieboy]

Hum, I haven't looked at this part of the code yet but do we really need to use C++ to handle such a basic thing?

To be honest, I'm against mixing C/C++ code unless we don't have the choice like for the cal3d lib. And making wrappers between C/C++ doesn't simplify the code IMO.

But well, it's just my opinion...

Another solution could be an array of pointers so you have less data to move or a linked list, and then malloc/strdup and free the memory.

 

The advantages of using a C++ wrapper are:

* Use C++ STL methods, which are already developed and rock-solid.

* Later migration to C++ code, if using same semantics.

 

We already have linked lists, with several flavours. We can have a "limited" linked list, which implements a pseudo-array, which can be iterated, appended, and limits the number of entries to a certain amount.

 

Or we can have a "char *[MAXLINES]; int low_watermark, high_watermark ", which can be faster if you onl need push()/pop() and shift()/unshift().

 

I can implement both approaches very quickly.

 

Álvaro

[LabRat]

I'll leave the rewrite to you lot then.

[alvieboy]

We're discussing the best way (and reusable way) to address this problem. I can (as you suggest) refactor the in-place code to use some "generic", "understandable" piece of code. And, if we agree I should to it, I will.

 

But this is not the point.

 

We lack some primitives right now, which can be fulfilled by the C++ STL. Again, if we do implement them in C (either directly or using wrapper code) and keep the semantics, we can later migrate to C++ code with not much hassle.

 

We also need to address the issue on the first place. The [console] code is so "hacked" right now no one understands clearly what it does - this is a consequence of patching and patching and patching, and forgetting about best practices and documentation. Using C++ STL / same semantics / would allow us to have functional code at one end, and generic (even specialized) code at the other end. Surely would benefit everyone.

 

If you look at my "largest" code in EL (popup), you'll see it uses a "Object Oriented" approach. This eases not only migration to a full OO language, but also simplifies the API you (and others) use to "instantiate" the functionality.

 

Saying "if you want it this way, do it yourself" doesn't help a lot.

 

It's not "me" that want it this way. If the "project" wants it this way, I shall follow the "project" decisions.

 

But again, we were only putting our cards on the table.

 

Álvaro

[LabRat]

I see several posts in succession since my post that (as far as I can see) say do it c++. I don't know c++ which I stated.

 

In C this would be a piece of cake for me - I would just change the pointers in memory when a new line came in rather than copy everything: &x = &(x[i-1]) and &x[last]=&justrecievedline would be all but instantaneous.

 

I have no clue what a std::vector is and frankly if whatever it is reads that complex I don't want to.

Edited April 2, 2008 by LabRat

[Cycloonx]

I have no clue what a std::vector is and frankly if whatever it is reads that complex I don't want to.

 

http://www.cplusplus.com/reference/stl/vector/

 

A vector (class) in c++ is nothing more than a "dynamic array" with some nice and easy to use functions. Actually it would help a lot in this case to hold the lines. But imo a queue would do the job better in this case (if size == 5000 pop the front away and push a new line).

 

Not that I want to say it's better for EL, it's just easier to use in general.

Edited April 2, 2008 by Cycloonx

[alvieboy]

I see several posts in succession since my post that (as far as I can see) say do it c++. I don't know c++ which I stated.

 

In C this would be a piece of cake for me - I would just change the pointers in memory when a new line came in rather than copy everything: &x = &(x[i-1]) and &x[last]=&justrecievedline would be all but instantaneous.

 

I have no clue what a std::vector is and frankly if whatever it is reads that complex I don't want to.

 

No.

 

Don't do it in C++. Do it in a "C++" approach.

 

an std::vector<X> is a container of objects of type X. You did C#, so you should be used to these generalizations.

 

Don't make code harder to understand. If you care about performance, inline your C functions. Otherwise, put some comments there explaining the hardest bits of it.

 

We're not to switch to C++, if thats what worries you. But, and if you're acostumated to pure OO languages, should be fairly easy.

 

Again, the "text.c", "console.c", "consolewin.c" is a huge mess. I'm just trying to reduce the complexity here.

 

Álvaro

[Vegar]

To get back to the topic - the reason an array is used is because the text is displayed in several text field widgets, and the use of an array simplifies things a lot (you just pass the array pointer to the widget). Using a queue, a vector, a linked list or anything else either requires a change to the text field widget (affects a lot of code) or a new widget that would be so similar to the text field widget that it just doesn't make any sense to do it.

 

There is nothing wrong with using an array, it's just done wrong here. As it is now, everything is a big mess, I don't think anyone can honestly say they understand all of the logic around the text buffer.

 

I suggest to keep the array, but rewrite most of the code around it. Instead of using an integer to track which message is the most recent (I think that's how it's done now), it would make a lot more sense to use memmove() to move old messages backwards in the buffer (similar to what labrat suggested).

Edited April 3, 2008 by Vegar

[alvieboy]

I suggest to keep the array, but rewrite most of the code around it. Instead of using an integer to track which message is the most recent (I think that's how it's done now), it would make a lot more sense to use memmove() to move old messages backwards in the buffer (similar to what labrat suggested).

 

memmove is pretty inneficient. However, since we don't have that many messages coming in might be a solution.

 

I still think using an array with ceil/tail indexes would be a lot faster, and safer if using functions to access the indexes, rather that doing calculations manually everywhere.

 

Álvaro

[Ermabwed]

Ok, found a funny bug... been playing for a while (adjusted font size to .98 so I actually see the last line), now with every new line I see the chatlog from the beginning below my chat...

Like this:

New text 1
old text 2

when I enter a new line, it goes one with

New text 1
New text 2
Old text 2

and so on

[Vegar]

If nobody have any objections, I would like to revert the commit referenced in the first post until a better solution is found. The fix is causing more problems than it's resolving, making the CVS client a pain to use.

Edited April 8, 2008 by Vegar

[alvieboy]

Hmm, I don't have any objection at all.

 

I really did not think much about this issue ,cause there seems to be no consensual approach for the fix/rewrite.

 

If we do agree on how to do it, I'll gladly hit my keyboard to fix it.

 

Álvaro

[Vegar]

Ok, I've reverted the fix.