search

clbr / radeontop

GNU General Public License v3.0
789 stars 69 forks source link

Improve dump output #115

Closed TilCreator closed 3 years ago

TilCreator commented 3 years ago

While implementing radeontop support into i3status-rust I noticed that the current dump output is not very nice to parse and is missing some information like vram/gtt max size and memory and shader clock max.

My solution for this is to add json as an additional dump format. I also updated the documentation a bit so that one doesn't have to read the source code before knowing what fields to expect in the output.

clbr commented 3 years ago

Sorry, JSON is an absolute no-no. I won't take this.

TilCreator commented 3 years ago

Could you at least explain what you dislike about JSON, what other data format you would accept and especially could you please not just close a PR because it has one (easily changeable) feature you don't like?

clbr commented 3 years ago

It's a horrible format that deserves an early death. Adding support for it would be implicitly supporting it and letting it gain market share.

No other dumping formats than the existing text will be accepted.

The PR's entire point was adding JSON support, it has no other changes.

TilCreator commented 3 years ago

You still have not explained why JSON and apparently every other data format, other than a custom format no one can work with, is "so bad" . Also this PR is called "Improve dump output", it not only adds JSON output, but also two fields to the custom dump format and much needed documentation.

MaxVerevkin commented 3 years ago

It's a horrible format that deserves an early death

But why? https://trends.google.com/trends/explore?date=all&q=%2Fm%2F05cntt,%2Fm%2F08745,%2Fm%2F01w6k2,%2Fg%2F11c5zwr35t

TilCreator commented 3 years ago

I know that it's popular, but why do you dislike it?

MaxVerevkin commented 3 years ago

I know that it's popular, but why do you dislike it?

Yeah, same question.

TilCreator commented 3 years ago

I still don't understand why you dislike popular data formats that help to connect different software?

How do you expect people to parse the dump output of radeontop?

MaxVerevkin commented 3 years ago

@TilCreator do you refer to me? I'm not the repository's owner.

TilCreator commented 3 years ago

Sry, my bad (got confused from the profile pic)

clbr commented 3 years ago

JSON is a text format unusable by humans, worst of both worlds. Human-understandable text formats are easily parsable, just because JSON is easier for $rust_app_x is no reason to support it. Hint: comma-separated string.

Adding other dump formats just because some app prefers to take JSON adds needless complexity into radeontop. "a custom format no one can work with" is slight hyperbole, or an indication you need more practice.

I'm sorry you wasted your time, but you could've asked first.

TilCreator commented 3 years ago

Human-understandable text formats are easily parsable

Just no, human language and everything around is exceptionally hard to parse.

Also compared to other data formats, JSON is very easy to read.

There is almost no programming language that doesn't support JSON, some even without any dependencies. So adding support for it would simplify connecting this utility to other software.

Isn't the dump output made for machine parsing and if not wouldn't it be preferable to have support for that?

Don't worry I will not make the mistake of trying to help you again.

clbr commented 3 years ago

The dump's primary use is script parsing, shell, awk, perl. Programming languages are almost as easy.

MaxVerevkin commented 3 years ago

Even though I disagree that JSON is a horrible format, I don't understand why was it so hard to parse current format?