Configurable Ping

[BourgeoisBear]

Lots of surgery on settings to get there. Let me know how it looks.

Thx.

JS

[BourgeoisBear]

Later commits were to address potential race conditions (esp in websocket.go). Given the nature of the program, it was unlikely that they would ever be triggered, but even so. If these are acceptable, I will probably add cmdline opts for specifying HTTP headers to send on websocket connect.

[BourgeoisBear]

I can take care of most of these this evening.

Will need to ponder the formatting in printToOut() though, & see if I can come up with something mutually pleasing. Have you tried the binary messages feature? There's always room for improvement, but the indent felt right for binary mode, and not everyone can see colors.

Thx.

JS

[thehowl]

Fair enough re: accessibility. Not thinking enough about accessibility is probably one of my deficiencies. I haven't had a chance to build & test yet, will do that on second round of review

[BourgeoisBear]

Applied the requested adjustments, aside from those regarding printToOut() -- need a day or so to ponder options there. On my local fork, I had a preliminary status bar implementation in editor.go. It's still there, but commented out for the last commit.

[BourgeoisBear]

not a github person. apparently i didn't want a "close"...

[BourgeoisBear]

just pushed the formatting adjustments

[thehowl]

Hi Jason, sorry for the very long tie period between the last activity on this last pull request and now. I didn't have immediate time to take care of this and I knew reviewing this batch of changes would have taken me a few hours, so I procrastinated this for a while longer than I'd like to admit. To you I offer my sincere apologies as I should have at least given an update on this PR, but in any case the PR is now merged into master.

I had some changes I wanted to see before this PR was merged; since such a long time has passed I've taken care of these myself. Including nitpicking work on code style, and some matters of personal preference that you had changed in the codebase. Most notably I found a solution that could be visually pleasing to my eyes while taking into consideration your suggestion about making the software accessible and usable even without the aid of colors. I'm not a fan of distinguishing this using indentation or not, and I found it visually asymmetric that the outbound messages were not indented while the inbound were not.

Ideally, a cool idea would be to have an optional "sidebar" on the message viewer which can act both as a line counter and a text indicator of which is in an inbound and which is an outbound message. Something like:

=> |
 2 |
 . |
 . |
 3 |
<= |
=> |
<= |

(where the dots are used to indicate lines without breaks that overflow).

However this approach is quite complex (because of handling screen resizes with wrapped lines, making it look nice, etc.) and I'm not even completely sure I can do it the way I want it to with gocui (which I'd need to get my hands dirty on again). So for now I went for a simpler solution, which though should enable visual distinctions effectively: the default format for timestamp now has a => prepended to it; this is changed to <=, == and !! as needed, based on the type of message. And both inbound and outbound messages are now indented; but only if timestamps are enabled, which thus becomes a kind of way also to use accessibility features.

Again, apologies this has taken so long. This was mostly because I knew properly addressing and reviewing your code would have taken a significant amount of time. I appreciate if you have other suggestions or contributions; however I'd ask you to keep them more concentrated and focused next time, so that also the diffs as a result are clear, as for this one I've had to keep quite a few tabs of my text editor and GitHub open at the same time, trying to understand where all the code was reshuffled.

Thanks! Morgan

[BourgeoisBear]

No problem. My use case had diverged so much from the adjustments I made to Claws that I rolled my own as a simple STDIN/OUT program instead of a TUI:

https://github.com/BourgeoisBear/wscli

But thank you for claws.

JS

On Jan 29, 2023, at 6:32 PM, Morgan @.***> wrote:

 Hi Jason, sorry for the very long tie period between the last activity on this last pull request and now. I didn't have immediate time to take care of this and I knew reviewing this batch of changes would have taken me a few hours, so I procrastinated this for a while longer than I'd like to admit. To you I offer my sincere apologies as I should have at least given an update on this PR, but in any case the PR is now merged into master.

I had some changes I wanted to see before this PR was merged; since such a long time has passed I've taken care of these myself. Including nitpicking work on code style, and some matters of personal preference that you had changed in the codebase. Most notably I found a solution that could be visually pleasing to my eyes while taking into consideration your suggestion about making the software accessible and usable even without the aid of colors. I'm not a fan of distinguishing this using indentation or not, and I found it visually asymmetric that the outbound messages were not indented while the inbound were not.

Ideally, a cool idea would be to have an optional "sidebar" on the message viewer which can act both as a line counter and a text indicator of which is in an inbound and which is an outbound message. Something like:

=> | 2 | . | . | 3 | <= | => | <= | (where the dots are used to indicate lines without breaks that overflow).

However this approach is quite complex (because of handling screen resizes with wrapped lines, making it look nice, etc.) and I'm not even completely sure I can do it the way I want it to with gocui (which I'd need to get my hands dirty on again). So for now I went for a simpler solution, which though should enable visual distinctions effectively: the default format for timestamp now has a => prepended to it; this is changed to <=, == and !! as needed, based on the type of message. And both inbound and outbound messages are now indented; but only if timestamps are enabled, which thus becomes a kind of way also to use accessibility features.

Again, apologies this has taken so long. This was mostly because I knew properly addressing and reviewing your code would have taken a significant amount of time. I appreciate if you have other suggestions or contributions; however I'd ask you to keep them more concentrated and focused next time, so that also the diffs as a result are clear, as for this one I've had to keep quite a few tabs of my text editor and GitHub open at the same time, trying to understand where all the code was reshuffled.

Thanks! Morgan

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.