cake-contrib / Cake.AddinDiscoverer

Tool to aid with discovering information about Cake Addins
MIT License
5 stars 6 forks source link

Add quotes to description of Cake extensions #189

Closed augustoproiete closed 3 years ago

augustoproiete commented 3 years ago

AddinDiscoverer is removing part of the description in the metadata for Cake.UrlLoadDirective.Module because it has the # character in it.

- Description: Cake module for supporting downloading files from url's in the #load directive.
+ Description: Cake module for supporting downloading files from url's in the 

We confirmed that if the description was emitted with quotes in it, it would resolve the issue:

- Description: Cake module for supporting downloading files from url's in the #load directive.
+ Description: "Cake module for supporting downloading files from url's in the #load directive."

Relates to https://github.com/cake-build/website/pull/1550#discussion_r570418850 and https://github.com/cake-build/website/pull/1883

Jericho commented 3 years ago

@pascalberger didn't you and I have a discussion about quotes and there was a reason you were not in favor of them? I vaguely remember you mentioning that quotes would have an impact on the web site generation process. In fact, I would have to go back in my history to double check but I think the discoverer used to include quotes and we decided to remove them. Is my memory wrong about this?

AdmiringWorm commented 3 years ago

Just my two cents.

There are two cases where you must have quotes (either single or double) in a yaml file.

  1. When there is a pound character in it (since it is used for comments)
  2. If the string either starts or ends with a star (*) (not sure about why on this one though).

In any other case, it should be fine that there are no quotes in the description string.

Jericho commented 3 years ago

Evidently, it's more complicated than that: https://www.yaml.info/learn/quote.html

augustoproiete commented 3 years ago

Is this a bug with YamlDotNet not emitting the quotes correctly when there's a # character? If yes, maybe as a workaround would be to use a custom EventEmitter similar to https://github.com/aaubry/YamlDotNet/issues/428#issuecomment-525822859 to force quotes to be emitted when the # character is present?

Jericho commented 3 years ago

Currently the discoverer contains custom logic to generate yaml files and also custom logic to determine if quotes are necessary or not around strings. It has become clear to me that the "quote" logic is much more complex that I originally thought and there are too many exceptions, edge cases, etc. for me to continue maintaining this custom logic. As @AdmiringWorm pointed out (and as the article I linked above explains in more details), there are a multitude of complicated scenarios that must be taken into account when deciding whether to "quote" a string or not, and the AddinDiscoverer is clearly not taking all these cases into account.

I think the best solution is to remove the custom logic from the discoverer and to rely on YamlDotNet's serialization logic. I did some testing and was able to observe that the quoting problem with strings containing # would go away, and also I discovered that there are cases where we unnecessarily quote strings such as this example: https://github.com/cake-build/website/blob/6f16bce991d18208ab5e925f8205e072616d7944/extensions/Cake.7zip.yml#L5

Good news/Bad news