Maybe there are two more testcases I can imagine (although the coverage is already at 100%):
removing some chars in the middle of the buffer (without backspace)
removing more chars in the middle of the buffer than there are (without backspace)
If you want to spend more time in this maybe some nonsense values can be tested (cursor outside of buffer size etc.). I guess this will break somehow ;)...
...maybe you should test the data in the write function as well with some assertions.
The remove_chars method writes the changed line back to the terminal depending on the echo setting in the instance. This can be a pain in the ass to test but might be a valuable test as well.
Just some suggestions. Personally I would have ended up with quite the same testcases ;)
But there is one more serious problem with the makefile indentation ;)
...does this still run on your machine?
on my system I get a "*** missing separator. Stop." error
As far as I know make needs the tab character as indentation in targets.
Would be nice if you revert this change ;)
But the rest is a good start.
Thanks :)
Thanks for collaborating :)
Testcases look nice!
Maybe there are two more testcases I can imagine (although the coverage is already at 100%):
1. removing some chars in the middle of the buffer (without backspace)
2. removing more chars in the middle of the buffer than there are (without backspace)
If you want to spend more time in this maybe some nonsense values can be tested (cursor outside of buffer size etc.). I guess this will break somehow ;)...
...maybe you should test the data in the write function as well with some assertions.
The remove_chars method writes the changed line back to the terminal depending on the echo setting in the instance. This can be a pain in the ass to test but might be a valuable test as well.
Just some suggestions. Personally I would have ended up with quite the same testcases ;)
But there is one more serious problem with the makefile indentation ;)
...does this still run on your machine?
on my system I get a "*** missing separator. Stop." error
As far as I know make needs the tab character as indentation in targets.
Would be nice if you revert this change ;)
But the rest is a good start.
Thanks :)
Needs more Work for testing the data in the write function.
Currently the unittest fails in the folowing Cases:
shellmatta_utils_removeChars_remove_chars_in_the_middle_of_the_buffer_backspace_false
shellmatta_utils_removeChars_remove_more_chars_in_middle_of_buffer_than_are_present_backspace_false
shellmatta_utils_removeChars_curser_outside_buffer
I Would like to suggest a change in utils_removeChars as follows:
if((0u != length) && (true == backspace))
{
/** -# rewind the cursor in case of backspace */
length = SHELLMATTA_MIN (length, inst->cursor);
utils_rewindCursor(inst, length);
/** -# delete the char at the cursor position */
[...]
I do not think there is any benefit in writing a unchanged string back to console anyway :).
Hopefully the indentation issue is fixed now as well - sorry for that!
Needs more Work for testing the data in the write function.
Currently the unittest fails in the folowing Cases:
shellmatta_utils_removeChars_remove_chars_in_the_middle_of_the_buffer_backspace_false
shellmatta_utils_removeChars_remove_more_chars_in_middle_of_buffer_than_are_present_backspace_false
shellmatta_utils_removeChars_curser_outside_buffer
I Would like to suggest a change in utils_removeChars as follows:
if((0u != length) && (true == backspace))
{
/** -# rewind the cursor in case of backspace */
length = SHELLMATTA_MIN (length, inst->cursor);
utils_rewindCursor(inst, length);
/** -# delete the char at the cursor position */
[...]
I do not think there is any benefit in writing a unchanged string back to console anyway :).
Hopefully the indentation issue is fixed now as well - sorry for that!
Maybe it was not clear enough what the function utils_removeChars shall do with backspace == false.
I tried to improve the documentation to make this clear.
Without backspace the function removes chars from the right hand side of the cursor, with backspace from the left hand side.
Due to the used VT100 like protocol it is not that easy to achive this :( - at least I do not know a better way than the implemented one:
move the cursor to the left (if backspace == true)
delete everything right of the cursor
print everything right of the cursor again.
Maybe this seems a bit overkill in the typical use case (delete one char with backspace) - so far I did not come across a better way. As far as I know there is no sequence for deleting chars - you have to clear the whole line or overwrite the chars.
I changed the implementation a bit to get more robust on invalid cursors. Please take a look at it. It should prevent invalid shifting of the buffer content.
Maybe it was not clear enough what the function utils_removeChars shall do with backspace == false.
I tried to improve the documentation to make this clear.
Without backspace the function removes chars from the right hand side of the cursor, with backspace from the left hand side.
Due to the used VT100 like protocol it is not that easy to achive this :( - at least I do not know a better way than the implemented one:
1. move the cursor to the left (if backspace == true)
2. delete everything right of the cursor
3. print everything right of the cursor again.
Maybe this seems a bit overkill in the typical use case (delete one char with backspace) - so far I did not come across a better way. As far as I know there is no sequence for deleting chars - you have to clear the whole line or overwrite the chars.
I changed the implementation a bit to get more robust on invalid cursors. Please take a look at it. It should prevent invalid shifting of the buffer content.
shimatta
Finally comming back to this - everything seems to build fine on my end, also i can not identify any problem with the identation.
Thanks for collaborating :) Testcases look nice!
Maybe there are two more testcases I can imagine (although the coverage is already at 100%):
If you want to spend more time in this maybe some nonsense values can be tested (cursor outside of buffer size etc.). I guess this will break somehow ;)... ...maybe you should test the data in the write function as well with some assertions. The remove_chars method writes the changed line back to the terminal depending on the echo setting in the instance. This can be a pain in the ass to test but might be a valuable test as well.
Just some suggestions. Personally I would have ended up with quite the same testcases ;)
But there is one more serious problem with the makefile indentation ;) ...does this still run on your machine? on my system I get a "*** missing separator. Stop." error As far as I know make needs the tab character as indentation in targets. Would be nice if you revert this change ;)
But the rest is a good start. Thanks :)
Needs more Work for testing the data in the write function.
Currently the unittest fails in the folowing Cases: shellmatta_utils_removeChars_remove_chars_in_the_middle_of_the_buffer_backspace_false shellmatta_utils_removeChars_remove_more_chars_in_middle_of_buffer_than_are_present_backspace_false shellmatta_utils_removeChars_curser_outside_buffer
I Would like to suggest a change in utils_removeChars as follows:
I do not think there is any benefit in writing a unchanged string back to console anyway :).
Hopefully the indentation issue is fixed now as well - sorry for that!
Maybe it was not clear enough what the function utils_removeChars shall do with backspace == false.
I tried to improve the documentation to make this clear. Without backspace the function removes chars from the right hand side of the cursor, with backspace from the left hand side.
Due to the used VT100 like protocol it is not that easy to achive this :( - at least I do not know a better way than the implemented one:
Maybe this seems a bit overkill in the typical use case (delete one char with backspace) - so far I did not come across a better way. As far as I know there is no sequence for deleting chars - you have to clear the whole line or overwrite the chars.
I changed the implementation a bit to get more robust on invalid cursors. Please take a look at it. It should prevent invalid shifting of the buffer content.