Option for closing tags list

[ostness]

Ace has predefined list of tag names to NOT be closed. It's handy to have an option to provide the list. If I'm producing XML, I'd have it empty.

Code changes notes:

• The list is kept in a map[string]struct{} as the size of struct{} is zero.
• Initializing func makes a copy of defaultNoCloseTagNames for opts.NoCloseTagNames, otherwise plain assignment would create a reference to the default (as it is a map) and it could be modified outside ace.

I also took the opportunity to make initializeOptions public. It becomes complex to re-do what the func does in non-ace code when it passes non-nil *ace.Options to ace.Load etc.

[yosssi]

@rzab Thanks for the nice PR! I'm curious why you decided to change the type of NoCloseTagNames from slice to map? Setting an empty struct to each key looks redundant and it seems that there's no merit of using map instead of slice because there's no code which access to map with its key.

[ostness]

It would be easier to modify .NoCloseTagNames as a map in non-ace code:

    aceopts := ace.InitializeOptions(nil)
    delete(aceopts.NoCloseTagNames, "input")

Adding a tag would be aceopts.NoCloseTagNames["input"] = struct{}{} which is ok in my book.

[ostness]

My thinking is this: ace cannot be sure about the format is produces. For anything strict (XML/XHTML) it's obvious the set must be empty. For HTML4 the existing default is ok, but not for HTML5. HTML5 has a dozen of tags which should not be closed, some of which are invalid in HTML4. Ace should not guess, it's up to user to deviate from the default set.

So as long as the set is an option, it must be easy for user to modify. delete is straight forward. Addition is maybe ugly because of struct{}{} but making it a map[string]bool so that the syntax would be better is just wasting space. We could add a helper public method for *ace.Options accepting []string or better yet ...string for the addition to the internal .NoCloseTagNames like this:

func (opts *Options) AddNoCloseTagNames(names ...string) {
    for _, name := range names {
        opts.NoCloseTagNames[name] = struct{}{}
    }
}
// Then we'd use it in `InitializeOptions':
if opts.NoCloseTagNames == nil {
    opts.AddNoCloseTagNames("br", "hr", "img", "input", "link", "meta")
}

Now imagine changing the set when it is a []string.

// Addition
aceopts.NoCloseTagNames = append(aceopts.NoCloseTagNames, "input")

// Deleting "input"
var newnoclose []string
for _, value := range aceopts.NoCloseTagNames {
    if value != "input" {
        newnoclose = append(newnoclose, value)
    }
}
aceopts.NoCloseTagNames = newnoclose

[yosssi]

Do you face the case in which you have to add or delete the element of NoCloseTagNames after you create ace. Options? I still cannot understand the necessity of the ability of modifying NoCloseTagNames because I cannot imagine the case when I have to modify NoCloseTagNames after the creation of ace. Options.

[ostness]

I am actually. Depending on a condition, I call ace.Load with either one set of arguments or another. So before any of the calls I setup common *ace.Options. And before the second call to ace.Load I redefine the .NoCloseTagNames, because, well, the result must be strict XML.

    aceopts := ace.InitializeOptions(nil)
    aceopts.FuncMap = templatep.AceFuncs

    if !jscriptMode {
        ace.Load(inputFile, definesFile, aceopts)
        return
    }

    delete(aceopts.NoCloseTagNames, "input")
    aceopts.AttributeNameClass = "className"
    ace.Load(definesFile, "", aceopts)

[yosssi]

@rzab Thanks for your explanation. I've thought this matter over and over again. Inside Ace, there's no need of making NoCloseTagNames's type map. So, I think it's good for us to leave its type be slice. Adding public helper functions such as (opts *Options) AddNoCloseTagName(name string) and (opts *Options) DeleteNoCloseTagName(name string) looks nice idea! They make the modification of NoCloseTagNames much easier.

[ostness]

Alright, slice it is.

[yosssi]

Thanks a lot!