Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uniform /status format #3220

Open
wants to merge 2 commits into
base: master
from

Conversation

@msjyoo
Copy link
Member

commented Jun 24, 2015

Remove trailing dot for uniform /status command for easier parsing

@msjyoo msjyoo added the C: Core label Jun 24, 2015

@shoghicp

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

Easier parsing, but you broke the existing parsing.

This was done to copy the format used in PC and other plugins (so existing code would be compatible with it), this now breaks compatibility with them completely.

@msjyoo

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2015

Are you serious right now, this is starting to feel like Microsoft on their backward compatibility motto.. (oh wait! :P)

Examples that this format is used in PC and will result in serious stuff braking?

@shoghicp

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

Anyway, this does not make parsing "easier", it's the same, just an extra character.
/^([^:]+): ([^\.]+)\.$/ with dot
/^([^:]+): (.+)$/ without dot

then remove the special chars and you have everything split by key and value

@msjyoo

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2015

sigh Why would you even make a commit with MBs with dots appended and Kb/s without a dot appended? This is a fix, not "breaking backwards compatibility"... I don't understand how an issue as critical as an unnecessary dot being appended taking up 4 extra bytes doesn't come across as an issue that requires IMMEDIATE fixing?!

It literally BURNS my eyes every time I run the status command, and I weep at the lack of uniformity between the lines how can you not see this as a problem it needs to be FIXED NOW!!!111

@msjyoo

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2015

Whoops, I seem to have dropped this -> /s

@msjyoo

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2015

@shoghicp Any update on this? It makes sense to drop the period.

@msjyoo msjyoo self-assigned this Aug 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.