* when not really really necessary we prefer to not touch other's code
You could always choose to ignore my patches. You could instead google my nickname
and checkup on my opensource experience.
Posted 31 July 2014 - 22:28
I think that part should be... ...so in fact it was wrong as it is now...
bool eListboxServiceContent::setCurrent(const eServiceReference &ref) { int index=0; for (list::iterator i(m_list.begin()); i != m_list.end(); ++i, ++index) if ( *i == ref ) { m_cursor = i; m_cursor_number = index; if (m_listbox) { m_listbox->moveSelectionTo(cursorResolve(index)); return true; } break; } return false; }
eh, that is exactly what the patch from captnoord is doing. So what is the problem? Technically it is not a functional change, at least not a change of the most probable intentional functionality. I.e. it is a bug fix!
Posted 31 July 2014 - 22:59
Edited by littlesat, 31 July 2014 - 23:02.
WaveFrontier 28.2E | 23.5E | 19.2E | 16E | 13E | 10/9E | 7E | 5E | 1W | 4/5W | 15W
Posted 1 August 2014 - 08:01
Posted 1 August 2014 - 09:57
@PLi team
did you add lcn support to your codes : https://github.com/o...fbf30d35384f1a5 ?
do we need it ?
Open Vision sources: https://github.com/OpenVisionE2
Posted 1 August 2014 - 10:05
Probably, BUT as always, this is probably DVB-T2 functionality, and that's not used here, so we cannot test it. So if you have the opportunity to test, please apply locally and test. If it's okay, we'll probably follow.did you add lcn support to your codes : https://github.com/o...fbf30d35384f1a5 ?
* Wavefrontier T90 with 28E/23E/19E/13E via SCR switches 2 x 2 x 6 user bands
I don't read PM -> if you have something to ask or to report, do it in the forum so others can benefit. I don't take freelance jobs.
Ik lees geen PM -> als je iets te vragen of te melden hebt, doe het op het forum, zodat anderen er ook wat aan hebben.
Posted 1 August 2014 - 10:14
FIxed mem leak because of mismatching allocation and deallocation in 'lib\gdi\picload.cpp'
see attached path file
Agree.
Committed.
Interesting fact: the delete statements were introduced by commit 2fb50a7e7447022319ad0d80349d5fc93b8b20dd "Memory leak and file handle leak fix" by Alex Revetchi <alex.revetchi@gmail.com> which was signed off by Littlesat...
* Wavefrontier T90 with 28E/23E/19E/13E via SCR switches 2 x 2 x 6 user bands
I don't read PM -> if you have something to ask or to report, do it in the forum so others can benefit. I don't take freelance jobs.
Ik lees geen PM -> als je iets te vragen of te melden hebt, doe het op het forum, zodat anderen er ook wat aan hebben.
Posted 1 August 2014 - 10:21
FIxed possible buffer overflow in 'lib/service/servicefs.cpp'
Committed.
Very well spotted, thx!
Edited by Erik Slagter, 1 August 2014 - 10:21.
* Wavefrontier T90 with 28E/23E/19E/13E via SCR switches 2 x 2 x 6 user bands
I don't read PM -> if you have something to ask or to report, do it in the forum so others can benefit. I don't take freelance jobs.
Ik lees geen PM -> als je iets te vragen of te melden hebt, doe het op het forum, zodat anderen er ook wat aan hebben.
Posted 1 August 2014 - 10:44
The last patch I am going to break up into smaller chunks with appropriate descriptions.
diff --git a/lib/service/listboxservice.cpp b/lib/service/listboxservice.cpp index edd03d7..b2793a2 100644 --- a/lib/service/listboxservice.cpp +++ b/lib/service/listboxservice.cpp @@ -88,15 +88,19 @@ bool eListboxServiceContent::setCurrent(const eServiceReference &ref) { int index=0; for (list::iterator i(m_list.begin()); i != m_list.end(); ++i, ++index) + { if ( *i == ref ) { m_cursor = i; m_cursor_number = index; if (m_listbox) + { m_listbox->moveSelectionTo(cursorResolve(index)); return true; + } break; } + } return false; }
The outer block only has one if statement inside, so it's not necessary to add braces. I think it's better though because it removes cause for confusion. This will be moved to a seperate commit with cosmetics.
The inner block, at second glance, I disagree about with pieterg, if you remove the indent, you end with two lines "return" and "break" after each other. This suggests it's not just the indent that's wrong. So applied.
@@ -112,7 +116,7 @@ int eListboxServiceContent::getNextBeginningWithChar(char c) { // printf("Char: %c\n", c); int index=0; - for (list::iterator i(m_list.begin()); i != m_list.end(); ++i, ++index) + for (list::iterator i = m_list.begin(); i != m_list.end(); ++i, ++index) { std::string text; ePtr<iStaticServiceInformation> service_info;
As far as I know this is purely a matter of style preferral. Also AFAIK used style is the only allowed style for "older" C++ standards.
while ( idx <= len ) { char cc = text[idx++]; - if ( cc >= 33 && cc < 127) + if ( isprint(cc) ) { if (cc == c) return index;
Applied.
Well-spotted, again. Missed the include though
@@ -465,8 +469,10 @@ int eListboxServiceContent::currentCursorSelectable() if (cursorValid()) { /* don't allow markers to be selected, unless we're in edit mode (because we want to provide some method to the user to remove a marker) */ - if (m_cursor->flags & eServiceReference::isMarker && m_marked.empty()) return 0; - return 1; + if (m_cursor->flags & eServiceReference::isMarker && m_marked.empty()) + return 0; + else + return 1; } return 0; }
Applied (cosmetic).
@@ -934,12 +940,14 @@ void eListboxServiceContent::paint(gPainter &painter, eWindowStyle &style, const // the progress data... eRect tmp = eRect(pb_xpos + m_progressbar_border_width, pb_ypos + m_progressbar_border_width, evt_done, m_progressbar_height); ePtr<gPixmap> &pixmap = m_pixmaps[picServiceEventProgressbar]; - if (pixmap) { + if (pixmap) + { painter.clip(tmp); painter.blit(pixmap, ePoint(pb_xpos + m_progressbar_border_width, pb_ypos + m_progressbar_border_width), tmp, gPainter::BT_ALPHATEST); painter.clippop(); } - else { + else + { if (!selected && m_color_set[serviceEventProgressbarColor]) painter.setForegroundColor(m_color[serviceEventProgressbarColor]); else if (selected && m_color_set[serviceEventProgressbarColorSelected]) @@ -948,13 +956,15 @@ void eListboxServiceContent::paint(gPainter &painter, eWindowStyle &style, const } // the progressbar border - if (!selected) { + if (!selected) + { if (m_color_set[serviceEventProgressbarBorderColor]) ProgressbarBorderColor = m_color[serviceEventProgressbarBorderColor]; else if (m_color_set[eventborderForeground]) ProgressbarBorderColor = m_color[eventborderForeground]; } - else { /* !selected */ + else /* !selected */ + { if (m_color_set[serviceEventProgressbarBorderColorSelected]) ProgressbarBorderColor = m_color[serviceEventProgressbarBorderColorSelected]; else if (m_color_set[eventborderForegroundSelected])
Not applied.
I don't think this is enough a coding style violation to warrant changing. This is what I meant when I mentioned we prefer not to touch other's code if it's not really necessary. Cosmetic changes make it harder to dig into the history of (functional) changes. Also many people prefer this style, so I guess it will show up at much more places in enigma's source.
* Wavefrontier T90 with 28E/23E/19E/13E via SCR switches 2 x 2 x 6 user bands
I don't read PM -> if you have something to ask or to report, do it in the forum so others can benefit. I don't take freelance jobs.
Ik lees geen PM -> als je iets te vragen of te melden hebt, doe het op het forum, zodat anderen er ook wat aan hebben.
Posted 1 August 2014 - 10:48
AFAIK all patches are processed now. Thanks captnoord for your contributions!
* Wavefrontier T90 with 28E/23E/19E/13E via SCR switches 2 x 2 x 6 user bands
I don't read PM -> if you have something to ask or to report, do it in the forum so others can benefit. I don't take freelance jobs.
Ik lees geen PM -> als je iets te vragen of te melden hebt, doe het op het forum, zodat anderen er ook wat aan hebben.
Posted 1 August 2014 - 10:58
else {looks totally confusing to me.
if { } else { }(my preference)
if { } else { }but not a mixture of } in its own line and { on same line ... never seen that anywhere before.
Posted 1 August 2014 - 12:40
Both agree with, BUT I repeat, don't change someone else's code if it's not really necessary. If only for git history's sake.
* Wavefrontier T90 with 28E/23E/19E/13E via SCR switches 2 x 2 x 6 user bands
I don't read PM -> if you have something to ask or to report, do it in the forum so others can benefit. I don't take freelance jobs.
Ik lees geen PM -> als je iets te vragen of te melden hebt, doe het op het forum, zodat anderen er ook wat aan hebben.
Posted 1 August 2014 - 14:40
Thanks Eric for applying my patches.
That's not a good way to make friends
* Wavefrontier T90 with 28E/23E/19E/13E via SCR switches 2 x 2 x 6 user bands
I don't read PM -> if you have something to ask or to report, do it in the forum so others can benefit. I don't take freelance jobs.
Ik lees geen PM -> als je iets te vragen of te melden hebt, doe het op het forum, zodat anderen er ook wat aan hebben.
Posted 1 August 2014 - 19:07
0 members, 30 guests, 0 anonymous users