[PATCH] Mark Air/Water temp field red for wrong input.

[Miika Turkia]

On Sun, Apr 13, 2014 at 10:10 AM, Yosef Hamza <jo.adam.93 at gmail.com> wrote:

> First I do know Regular expression.
>
> Second What I wanted to make is "ONE" character margin of error -If you
> changed the text further with that error it will give red-.
> That wasn't my first solution to that problem, I was careful about keeping
> the functionality while adding that 1 character margin of error -but if
> edited further with that error it will mark it red-.
>

Ah, in that case this code makes sense. Should probably include the
reasoning in the commit message as it was not clear from the code (at least
I missed it completely).


> The regex solution came to mind but wasn't exactly what I wanted.
>
> The Question is what I wanted was the right approach for that problem
> -adding one character error margin-? and if so is it worth it?
>

This is (IMHO) better than the regex I first had in mind. In big picture
they both work well as the actual parsing of the input value discards the
possible errors allowed by my regexp.

Currently you have still a slight inconsistency. If one starts typing with
plus or minus sign, then an error is indicated. This is cleared when adding
a digit.

I would also rather have one validation function instead of having the same
code twice. Just call the validation function from both of these locations.

I also spotted two spelling errors:
invaild => invalid
percision => precision

miika

On Sun, Apr 13, 2014 at 7:04 AM, Miika Turkia <miika.turkia at gmail.com>wrote:

> On Sat, Apr 12, 2014 at 8:42 PM, Yosef Hamza <jo.adam.93 at gmail.com> wrote:
>
>> Thanks for the review.
>>
>> I made it a little more tolerant if editing existing values instead of
>> giving red while deleting it at some point -after deleting the Unit and
>> after last number after the '.'-  then back to normal yellow and for empty
>> string -or should I leave it marked red for empty string?- which made it
>> more consistent for the user.
>>
>
> It can be made more tolerant also by modifying the regular expression.
> Which I think should be done instead of adding more if statements. I agree
> that empty value should not be marked as red. The following regular
> expression should be the same as your if statements (or at least very close
> to it).
>
>     if
> (!text.contains(QRegExp("^[-+]{0,1}[0-9]*([,.][0-9]*){0,1}([°]{0,1}[CF]{0,1}){0,1}$")))
> {
>         QPalette p;
>         p.setBrush(QPalette::Base, QColor(Qt::red).lighter());
>         ui.airtemp->setPalette(p);
>     }
>
> The regular expression above will keep everything yellow, if it is
> possible that completed syntax will be yellow (as your added if statements
> would do).
>
> If you need to modify the regular expression, here is a quick run-down:
> [asdf]      Any one of the characters mentioned inside brackets
> [-+]{0,1}   This will allow 0 or 1 characters in the previous grouping, in
> this case inside the angler brackets
> [0-9]*       Allows any number in the previous grouping
> [0-9]+      Would allow any number containing at least one digit in the
> previous grouping
> (°C){0,1}   Allows text "°C" 0 or 1 times, you can use parentheses for
> grouping
>
> Also your coding style is inconsistent and is not following our coding
> style:
>
> +       }
> +               else{
> +                       missing_unit =false;
>
> Else should be on same line with closing }
> There should be space between if or else statement and {
> Space between = and the following text missing
>
> Take a look at file "CodingStyle" in main directory of our source code.
>
> miika
>


>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140413/89969ae1/attachment-0001.html>