Rotating dash not removed

[egmont]

When reading a large directory, mc shows a rotating dash in the upper right corner (if enabled in the config). This symbol is not removed when mc finishes reading the directory. (It disappears on a subsequent Ctrl-R or window resize, though).

The easiest way to test it: stand on a directory, press and hold the Enter key for a long time (mc toggles back and forth between two directories).

When mc finishes reading a directory, it should remove the rotating dash.

[egmont]

remove rotating dash when finished rotating

[egmont]

This has been bugging me forever, and I don't know why I didn't take time to fix it much earlier. My system is fast enough that I rarely see the dash actually rotating, but I'm always disturbed by the artifact it leaves behind, it just looks ugly to me.

Could you please review the patch?

Whenever rotating the dash finishes, I redraw the character that should be there. I believe there are 2 cases only: if the menubar is shown (then it's a space char, using menubar color), or if it's hidden (then it's the upper right corner of the panel, using the main color).

[As a really minor bug, if the menubar is shown but the terminal is way too narrow to show it fully (e.g. less than 42 chars wide in English), I print a space although a letter (the last visible one from the cropped menubar line) should be printed. Honestly I don't care about it as mc is useless anyways on such narrow terminals :) ]

As a side note, I inverted the boolean arg of find_rotate_dash() to be consistent with the new rotate_dash() behavior and to be intuitive. For me, FALSE meaning to show something and TRUE meaning to hide it was counter-intuitive. I couldn't figure out where this rotating dash appears, though.

[andrew_b]

Replying to egmont:

Could you please review the patch?

In general, patch is OK. Thanks!

Whenever rotating the dash finishes, I redraw the character that should be there. I believe there are 2 cases only: if the menubar is shown (then it's a space char, using menubar color), or if it's hidden (then it's the upper right corner of the panel, using the main color).

[As a really minor bug, if the menubar is shown but the terminal is way too narrow to show it fully (e.g. less than 42 chars wide in English), I print a space although a letter (the last visible one from the cropped menubar line) should be printed. Honestly I don't care about it as mc is useless anyways on such narrow terminals :) ]

I moved dash to the corner of the right/top panel from the corner of the screen. Menubar is untouched now.

As a side note, I inverted the boolean arg of find_rotate_dash() to be consistent with the new rotate_dash() behavior and to be intuitive. For me, FALSE meaning to show something and TRUE meaning to hide it was counter-intuitive. I couldn't figure out where this rotating dash appears, though.

This part of your patch I applied with small modifications as a separate commit.

Branch: 2163_rotating_dash.
Initial changeset:941f9a279a596bd9c11ebc73aa6e780999fb6c3a.

[egmont]

Thanks! One thing, though, please do

- pos = (pos + 1) % sizeof (rotating_dash); 
+ pos = (pos + 1) % (sizeof (rotating_dash) - 1); 

sizeof() counts the terminating zero, hence a 5th phase, a caret also shows up.

I'm not sure about the widget infrastructure of mc, but are you sure that using midnight_dlg does not interfere with any actual dialog that might be open while the dash is rotated? (If you're sure then that's okay! :))

[andrew_b]

Replying to egmont:

sizeof() counts the terminating zero

Indeed.
In case of

const char rotating_dash[] = "|/-\\";

sizeof (rotating_dash) == 5.

In case of

const char *rotating_dash = "|/-\\";

sizeof (rotating_dash) == 4.

I changed declaration of rotating_dash.

I'm not sure about the widget infrastructure of mc, but are you sure that using midnight_dlg does not interfere with any actual dialog that might be open while the dash is rotated? (If you're sure then that's okay! :))

I hope everything is OK here. :)

[egmont]

I believe in case of

char *rotating_dash = "|/-\\";

sizeof(rotating_dash) accidentally happens to be 4 because the size of a pointer is 4 bytes, and not because the length of the string contained there is 4.

[egmont]

Yup, sizeof(rotating_dash) == 8 on x86-84.

Please stick to the [] declaration (that's when the length can be known in compile time) and the -1 offset. Or the hardwired 4 :) Or you can do

char rotating_dash[4] = "|/-\\"

in which case the trailing zero is not stored in memory at all and then sizeof gives 4, but you still have to manually maintain the length in that declaration line.

In case of "char *" the length might change at runtime hence sizeof() (which is evaluated at compile time) has no chance of knowing it, and it would be weird if the "const" modifier changed the behavior of sizeof().

[andrew_b]

Replying to egmont:

Please stick to the [] declaration (that's when the length can be known in compile time) and the -1 offset. Or the hardwired 4 :) Or you can do

char rotating_dash[4] = "|/-\\"

in which case the trailing zero is not stored in memory at all and then sizeof gives 4, but you still have to manually maintain the length in that declaration line.

Ok. Thanks.

[egmont]

It's cool now, thx!

[zaytsev]

You guys are doing awesome work! Just so that you know...

[mike.dld]

It is still possible to get sizeof() == 4, not hard-code any magic numbers and make the variable immutable at the same time. The code would look like this:

const char rotating_dash[] = { '|', '/', '-', '\\' };

[egmont]

Good point, Mike!

[andrew_b]

Merged to master: [d38589cbcaa9f4e72dd2cadc4a96d29deab9e94b].

git log --pretty=oneline 368a303..d38589c

[egmont]

Unfortunately the fix is not complete yet. The rotating dash is now drawn by using a widget which is not resized when mc is resized. So if you make mc's terminal wider, the rotating dash and the box corner when it's removed is drawn at an incorrect position.

[andrew_b]

Strange.
It should be fixed in [a365be63a29b5f78766c0aef7e0b9fe80c258d82].

[egmont]

Oops, sorry. My bad. It is indeed fixed.

(I forgot that I had downgraded to 4.8.9 because #3047 was driving me crazy.)