efficios / barectf

Generator of ANSI C tracers which output CTF data streams
https://barectf.org
MIT License
65 stars 17 forks source link

RFC: Remove the '...' at the end of the effective configuration output #26

Open ebugden opened 1 year ago

ebugden commented 1 year ago

The "show-effective-config" command prints the entire configuration object that will be used by barectf, however the '...' that appears at the end of the output implies continuation. It suggests the output was cropped or is incomplete.

In my case, after using the command I went to check the YAML configuration file to make sure the end of the command output matched the end of the file.

Remove the '...' to clarify that the command outputs the complete configuration object.

image

eepp commented 1 year ago

This is valid YAML; see Document Markers.

Three dots means "end of document" and confirms that it's complete and nothing more is to be expected.

ebugden commented 1 year ago

Ah ok! Is there a functional reason for displaying the "document end marker" at the end of the effective configuration output? My understanding is that the document markers are to avoid ambiguity about where files start/end when parsing multiple YAML files in sequence: "YAML uses three dashes (“ --- ”) to separate documents within a stream. Three dots ( “ ... ”) indicate the end of a document without starting a new one, for use in communication channels.", but in this case the command's goal seems to only be to clearly communicate the configuration object to the user.

My impression is that in practice '...' aren't often used in YAML files (some quick sources: (1) most yaml tutorials don't seem to include them in their file examples, (2) comment, (3) @mjeanson has never seen this end marker in a YAML file before) and that most users won't understand that '...' explicitly indicates the end of a file.

If the command goal is to clearly communicate the configuration to the average user, if it's quick and easy to remove '...' from the output, and if this change has no negative functional impact on barectf, I think removing it would make things more clear for most folks using the command.

That being said, here are some caveats that could make this change less relevant:

eepp commented 1 year ago

No functional reason!

I believe I just opted for the most complete YAML document.

If:

Then I'll accept a Gerrit change for the ˋmaster` branch.

ebugden commented 1 year ago

Sounds good! How would you like me to confirm that it's useless? That the project tests pass? That this command output is unlikely to be used in scripts that this change could break? I can also make a reasonable guess and you can let me know later if I missed something.

Also what do you mean by "still valid without the dots"? That the output is a valid YAML file? (Is a goal also that users can copy-paste the output of that command into a file and use that as their config?)

eepp commented 1 year ago

Sounds good! How would you like me to confirm that it's useless? That the project tests pass? That this command output is unlikely to be used in scripts that this change could break? I can also make a reasonable guess and you can let me know later if I missed something.

I guess removing those dots could break something somewhere, but I'm confident that it won't. So I'd just do it and wait for bug reports which will probably never exist.

Also what do you mean by "still valid without the dots"? That the output is a valid YAML file? (Is a goal also that users can copy-paste the output of that command into a file and use that as their config?)

Yes that's the purpose of this command, especially when reading a barectf 2 config.

ebugden commented 1 year ago

ebugden: Is a goal also that users can copy-paste the output of that command into a file and use that as their config?

eepp: Yes that's the purpose of this command, especially when reading a barectf 2 config.

Oh! So would the main purposes of this command be:

Asking so that I don't propose changes that break the intended use cases.

eepp commented 1 year ago

Yes exactly.