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

do not indent leaf tag. #26

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

do not indent leaf tag. #26

wants to merge 2 commits into from

Conversation

mgi
Copy link
Contributor

@mgi mgi commented Dec 8, 2017

When indenting, tags in the form (:tag "content") are printed like
this:

<tag>content</tag>

instead of:

<tag>content
</tag>

When indenting tags in the form (:tag "content") are printed like
this:

<tag>content</tag>

instead of:

<tag>content
</tag>
@mgi
Copy link
Contributor Author

mgi commented Dec 8, 2017

Maybe this should be optional...

@stassats
Copy link
Member

stassats commented Dec 8, 2017

Maybe it should depend on how long "content" is?

@mgi
Copy link
Contributor Author

mgi commented Dec 8, 2017 via email

This work for static content only. Not for computed content as:
```
(let ((str (make-string 200 :initial-element #\a))) (:tag (who:str str)))
```
@mgi
Copy link
Contributor Author

mgi commented Dec 8, 2017 via email

@mgi
Copy link
Contributor Author

mgi commented Jan 23, 2018

Hi,
Do you think it can go in? Or should it be closed and disabled?

@stassats
Copy link
Member

This is a low priority enhancement, I'll look at it when I have time.

@mgi
Copy link
Contributor Author

mgi commented Jan 24, 2018

Ok, I understand that. Get back to sbcl ;-)

@@ -120,6 +120,9 @@ in *HTML-EMPTY-TAGS* as <tag/> \(XHTML mode) or <tag> \(SGML
mode and HTML5 mode). For all other tags, it will always generate
<tag></tag>.")

(defvar *short-leaf-content-length* 72
"Below this threshold leaf tags won't be indented.")
Copy link

Choose a reason for hiding this comment

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

This docstring is unclear in that it doesn't specify units. I'd much prefer something like "Leaf tags shorter than this many characters won't be indented."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow, old stuff! Yes fine by me. Having units is better. I'm also ok with your other comments… but now I should find out how to modify the patch for this PR :-|

a list of strings or Lisp forms. LEAF-P is t when the TAG is a leaf
and is not indented."))
a list of strings or Lisp forms. SHORT-LEAF-P is t when the TAG is a
leaf and its content is shorter than *SHORT-LEAF-CONTENT-LENGTH* then
Copy link

Choose a reason for hiding this comment

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

Again, specifying that this is in characters would be useful. Also, the grammar here irks me.

"[...] is t when TAG is a leaf and its content is shorter than SHORT-LEAF-CONTENT-LENGTH characters. Short leaves are not indented."


(defmethod convert-tag-to-string-list (tag attr-list body body-fn leaf-p)
(defmethod convert-tag-to-string-list (tag attr-list body body-fn short-leaf-p)
Copy link

Choose a reason for hiding this comment

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

This is an awful lot of required params. Could some of these be &key args?

(< (loop for e in (rest s) when (stringp e) sum (length e))
*short-leaf-content-length*))))
(let (tag attr-list body)
(cond
Copy link

Choose a reason for hiding this comment

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

incorrect indentation of cond clauses

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