Skip to content

Merge in "verbose" mode#31

Open
nwwells wants to merge 3 commits into
websockets:masterfrom
truhlikfredy:master
Open

Merge in "verbose" mode#31
nwwells wants to merge 3 commits into
websockets:masterfrom
truhlikfredy:master

Conversation

@nwwells

@nwwells nwwells commented Jan 24, 2017

Copy link
Copy Markdown
Collaborator

@truhlikfredy, what do you think about adding a command-line flag for verbosity?

@truhlikfredy

Copy link
Copy Markdown

Yes that would be better, because now does only hardcoded console.log()

@3rd-Eden

Copy link
Copy Markdown
Member

This PR also contains renaming of package, bin and what not. Ideally verbose mode should be locked behind a -v CLI option.

@nwwells

nwwells commented Jan 25, 2017

Copy link
Copy Markdown
Collaborator Author

Right, @3rd-Eden. I was just about to ask, @truhlikfredy, would you be willing to add the flag and remove all the other changes?

@truhlikfredy

Copy link
Copy Markdown

I didn't initiated the PR (i think my fork is not good enough in this stage for merge). The changes made there are very crude and dirty. Renamed the package so it will not overwrite the original when original behavior is needed.

@lpinca

lpinca commented Jan 26, 2017

Copy link
Copy Markdown
Member

I'm -1 on these changes. I think there is no real value in printing the flag objects.
I would like to see them removed from ws completely but that's another issue.

@nwwells

nwwells commented Jan 26, 2017

Copy link
Copy Markdown
Collaborator Author

This brings up an interesting question, though: What is the expected behavior for binary data? netcat of course just passes it through, but the ws protocol provides the concept of frames, which are converted into \ns by wscat. You could base64 encode binary data (or hex encode, as with #5), what do you guys think of that, @lpinca & @3rd-Eden?

@lpinca

lpinca commented Jan 26, 2017

Copy link
Copy Markdown
Member

I think that if the 'message' listener receives a buffer it makes sense to print it as hex.

wsConsole.print(`< ${buf.toString('hex')}`);

Something like this, beautified if necessary. It probably wasn't designed to deal with binary messages. What other tools do in this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants