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

Add HTML escape function #143

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add HTML escape function #143

wants to merge 2 commits into from

Conversation

eloycoto
Copy link

@eloycoto eloycoto commented May 4, 2022

When running yaml files, some data inside the yaml can break the HTML
tables, and there is no way to escape the chars natively.

This commits adds a way to escape variable:

{{escape .Description}}

Signed-off-by: Eloy Coto [email protected]

@eloycoto eloycoto requested a review from roee88 as a code owner May 4, 2022 08:21
@roytman
Copy link
Contributor

roytman commented May 4, 2022

Hi @eloycoto, thanks for the PR.
I tried it, but looks like the escaping doesn't work.
I replaced

description: Some area
to "Some ></# area"
The result was

<td>
   Some ></# area<br/>
   <br/>
       <i>Enum</i>: East, West<br/>
</td>

Of course, it is not correct HTML.
Did I miss something?

@eloycoto
Copy link
Author

eloycoto commented May 5, 2022

yeah, works for me:

diff --git a/example/crds/example.yaml b/example/crds/example.yaml
index 6d0b33c..3fd26dc 100644
--- a/example/crds/example.yaml
+++ b/example/crds/example.yaml
@@ -34,6 +34,12 @@ spec:
                     default: 7
                     minimum: 0
                     maximum: 10
+                  sample:
+                    description: 'Selects a field of the pod:
+                      supports metadata.name, metadata.namespace,
+                      `metadata.labels[''<KEY>'']`, `metadata.annotations[''<KEY>'']`,
+                      spec.nodeName, spec.serviceAccountName,
+                      status.hostIP, status.podIP, status.podIPs.'
                   text:
                     description: Just text
                     type: string
diff --git a/example/output.md b/example/output.md
index 32596bb..8cba092 100644
--- a/example/output.md
+++ b/example/output.md
@@ -1,4 +1,7 @@
-# API Reference
+---
+title: API Reference
+weight: 1
+---
 
 Packages:
 
@@ -59,7 +62,7 @@ Resource Types:
         </td>
         <td>false</td>
       </tr><tr>
-        <td><b><a href="#examplemessagesindexkey">messages</a></b></td>
+        <td><b><a href="examplemessagesindexkey">messages</a></b></td>
         <td>[]map[string]object</td>
         <td>
           <br/>
@@ -70,7 +73,7 @@ Resource Types:
 
 
 ### Example.messages[index][key]
-<sup><sup>[↩ Parent](#example)</sup></sup>
+<sup><sup>[↩ Parent](example)</sup></sup>
 
 
 
@@ -106,6 +109,13 @@ Resource Types:
             <i>Maximum</i>: 10<br/>
         </td>
         <td>false</td>
+      </tr><tr>
+        <td><b>sample</b></td>
+        <td></td>
+        <td>
+          Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels[&#39;&lt;KEY&gt;&#39;]`, `metadata.annotations[&#39;&lt;KEY&gt;&#39;]`, spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.<br/>
+        </td>
+        <td>false</td>
       </tr><tr>
         <td><b>text</b></td>
         <td>string</td>
@@ -114,4 +124,4 @@ Resource Types:
         </td>
         <td>false</td>
       </tr></tbody>
-</table>
\ No newline at end of file
+</table>
diff --git a/templates/frontmatter.tmpl b/templates/frontmatter.tmpl
index 351cc2d..94f3b58 100644
--- a/templates/frontmatter.tmpl
+++ b/templates/frontmatter.tmpl
@@ -71,7 +71,7 @@ Resource Types:
         <td><b>{{if .TypeKey}}<a href="{{.TypeKey}}">{{.Name}}</a>{{else}}{{.Name}}{{end}}</b></td>
         <td>{{.Type}}</td>
         <td>
-          {{.Description}}<br/>
+          {{ escape .Description }}<br/>
           {{- if or .Schema.Format .Schema.Enum .Schema.Default .Schema.Minimum .Schema.Maximum }}
           <br/>
           {{- end}}

@roytman
Copy link
Contributor

roytman commented May 5, 2022

@eloycoto , from your output I see changes in /templates/frontmatter.tmpl, but they are not part of the PR. (btw, there are several template files).

I changed the fronmatter.tmpl file accordingly, now it works.
Looks like the template files should be updated too.

@eloycoto
Copy link
Author

eloycoto commented May 9, 2022

Looks like the template files should be updated too.

I'm happy to update that, but I think that it's more a corner cases than anything ;-) For me is more have a way to have that function enabled.

When running yaml files, some data inside the yaml can break the HTML
tables, and there is no way to escape the chars natively.

This commits adds a way to escape variable:

```{{escape .Description}}```

Signed-off-by: Eloy Coto <[email protected]>
@eloycoto eloycoto changed the title Add HTML ecape function Add HTML escape function May 16, 2022
@roee88
Copy link
Contributor

roee88 commented May 17, 2022

@roytman I don't think that escaping should be in the template as it actually makes sense to use HTML tags in the description (e.g., line breaks).

@roytman
Copy link
Contributor

roytman commented May 17, 2022

so you guys think that we need to provide a possibility to escape the special characters, and if somebody wants to do it, he will update the relevant template, correct?
The problem is that most of the users use the default template without any changes.

@roytman
Copy link
Contributor

roytman commented May 17, 2022

( I'll fix the tests)

Copy link
Contributor

@roytman roytman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @eloycoto , sorry for misleading, but we do need unescaped new lines
in some comments.
You were right about a corner case when you suggested defining the escape function only, without updating the templates.

Could you please return to your original commit.

@@ -1,4 +1,4 @@
module fybrik.io/crdoc
module github.com/eloycoto/crdoc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please return back the module name

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.

3 participants