x.websocket: Another try for V docs style of comments :) #7091
Conversation
helto4real
changed the title
| handler SocketMessageFn // callback function | ||
| handler2 SocketMessageFn2 // callback function with reference | ||
| is_ref bool // true if has a reference object | ||
| ref voidptr // referenced object |
There was a problem hiding this comment.
@medvednikov are field doc comments behind the field or above? would it be on the same line like [attributes]? then the line can be get really long...
There was a problem hiding this comment.
Yea interesting question. I could not find clear rules about that. Fmt should handle it preferable
|
You need a global search and replace to change |
Fixed... I need to learn how to spell that :) |
If you want to help me review the planned refactor moving from x module to net module I will go through naming of naming. Specifically private functions and make them clearer. Many of them are from the original code. Better names I can remove alot of those comments as well for private functions. |
|
i think everyone here can help, i haven't used the websocket module once. |
|
maybe we can make renaming in an extra PR with an issue which collects all naming issues. |
Made issue and assigned myself #7098. |
| pub fn (mut ws Client) on_close(fun SocketCloseFn) { | ||
| ws.close_callbacks << CloseEventHandler{ | ||
| handler: fun | ||
| } | ||
| } | ||
|
|
||
| // on_close_ref, register a callback on closed socket and provide a reference | ||
| // on_close_ref registers a callback on closed socket and provides a reference object |
There was a problem hiding this comment.
From a user perspective I'm a little unsure what the "reference object" is - and what it does?
It's mentioned quite a few times throughout the code.
My guess is that it's a way for the user to provide her or his own context, right?
Maybe it could be explained in detail on one of the structs?
| @@ -4,7 +4,7 @@ import net | |||
| import time | |||
| import sync | |||
|
|
|||
| // socket_read reads into the provided buffer with its length | |||
| // socket_read reads from socket into the provided buffer | |||
| fn (mut s Server) handle_server_handshake(mut c Client) ?(string, &ServerClient) { | ||
| msg := c.read_handshake_str() ? | ||
| handshake_response, client := s.parse_client_handshake(msg, mut c) ? | ||
| unsafe {msg.free()} | ||
| return handshake_response, client | ||
| } | ||
|
|
||
| // parse_client_handshake parses handshake result | ||
| // parse_client_handshake parses result from handshake process |
| @@ -35,15 +33,15 @@ fn (mut ws Client) handshake() ? { | |||
| unsafe {handshake_bytes.free()} | |||
| } | |||
|
|
|||
| // handle_server_handshake manage websocket server handshake | |||
| // handle_server_handshake manages websocket server handshake process | |||
| handler SocketCloseFn // callback function | ||
| handler2 SocketCloseFn2 // callback function with reference | ||
| is_ref bool // true if has a reference object | ||
| ref voidptr // referenced object |
There was a problem hiding this comment.
See my other comment about what the referenced object actually is
| @@ -52,7 +50,7 @@ pub fn (mut s Server) set_ping_interval(seconds int) { | |||
| s.ping_interval = seconds | |||
| } | |||
|
|
|||
| // listen, start listen to incoming connections | |||
| // listen start listen and process to incoming connections from websocket clients | |||
There was a problem hiding this comment.
// listen starts listening and process incoming connections from websocket clients
| @@ -67,8 +65,9 @@ pub fn (mut s Server) listen() ? { | |||
| s.logger.info('websocket server: end listen on port $s.port') | |||
| } | |||
|
|
|||
| // Close server (not implemented) | |||
| // Close closes server (not implemented yet) | |||
| @@ -105,7 +104,7 @@ fn (mut s Server) handle_ping() { | |||
| } | |||
| } | |||
|
|
|||
| // serve_client accepts incoming connection and setup the websocket handshake | |||
| // serve_client accepts incoming connection and sets up the callbacks | |||
| @@ -159,7 +158,7 @@ fn (mut s Server) setup_callbacks(mut sc ServerClient) { | |||
| }, sc) | |||
| } | |||
|
|
|||
| // accept_new_client creates a new client instance for client connects to socket | |||
| // accept_new_client creates a new client instance for client that connects to the socket | |||
| @@ -181,7 +180,7 @@ fn (mut s Server) set_state(state State) { | |||
| } | |||
| } | |||
|
|
|||
| // free, manual free memory of Server instance | |||
| // free manages manual free of memory for Server instance | |||
There was a problem hiding this comment.
// free frees all memory allocated by the Server instance
|
Sorry to grammer-slam you @helto4real - your work is much appreciated! 😅 |
No worries, I was thinking the same. This is why I took action to make it better. Lets make this close to how the quality bar of docs should be. I am greatful since English is not my native language obviously ;) It’s the final result that counts! |
|
I think that is going to be one of the most commented PRs in this repository 😄 . |
|
Heh. I think it's at the point where it should probably be merged, and any lingering issues can be cleaned up after the |
Second attempt to comply to V docs style (go docs). #7047