habitat-sh / habitat

Modern applications with built-in automation
https://www.habitat.sh
Apache License 2.0
2.6k stars 314 forks source link

[discussion] Should template files have an extension? #1183

Closed miketheman closed 7 years ago

miketheman commented 8 years ago

It is common practice to name files with an extension appropriate to their contents.

A snippet from Wikipedia:

Template file formats are those whose file extension indicates that the file type is intended as a very high starting point from which to create other files.

The author of the handlebars crate used in habitat uses a file extension to denote the templated nature of the file.

When using other tools - such as code quality linters - against a habitat plan, we can prevent confusing behaviors by having the filename extension denote what the contents are expected to be.

The Handlebars project appears to have adopted a couple of extensions:

(The latter is too long to get my vote, the former being "just right".)

I'd like to discuss what would be involved in extending the handlebars helpers to support the .hbs files extension (and remove it when rendering it) to templates to help avoid confusion as to what is a template, and what is not, and whether or not this has any significance to anyone but me.

bookshelfdave commented 8 years ago

I like this, I've been confused on several occasions regarding "which files are templates?". If we ever support additional/replacement template formats (which isn't being discussed or planned), it would help identify templates that would be affected.

This would help the issue of swap/tmp files in the config directory blowing up the Handlebars parser as well: https://github.com/habitat-sh/habitat/issues/1120

Template rendering code:

juliandunn commented 8 years ago

I'd love to get some users' votes on this. Can you react to this message with 👍 if you'd like us to force an extension like .hbs for templates (and ignore anything that doesn't have this extension) or 👎 if you disagree?

Obviously, this would be a breaking change.

eeyun commented 8 years ago

I dig this!

adamhjk commented 8 years ago

I'll be the lone voice against. :)

1) Supporting multiple template options should be a mammoth hill to climb, in terms of what would make us accept such a PR. It would introduce a huge amount of user complexity, not to mention documentation and guidelines. For now, it's handlebars, and unless something so compelling comes up, it'll be handlebars forever.

2) Given that it's one format, and always in a directory, "which are templates" should already be clear - anything in that directory is a template. I can no longer take a raw configuration file, copy it in that directory, and move on. Instead, I have to rename it to (.hbs), and I have to explain why.

3) The right behavior is to reject binary files altogether from templating, because we cannot effectively parse them. The secondary question is whether we should copy them as-is (probably). We couldn't ignore the existence of files in the configuration directory, because we have to assume your intent it to utilize them for configuration.

So I'm a pretty big 👎 on this, but I won't block the will of the universe. I think it adds complexity more than it removes it.

miketheman commented 8 years ago

Dissent drives discussion, and the need for clarity, which is great!

1) I don't think I was asking to support any more templating options - sorry if that was misconstrued - only to continue to support handlebars with a known file extension.

2) The part of "which are templates" is what the file extension denotes - and I believe that if we proceed with this, then we can approach the solution of "if a file does not have a template extension, don't treat it like a template".

3) I don't think it's common to place binary files in config directory, but it's not impossible. Hence "if it's a template, let's name it with a template extension".

Does that clarify my initial proposal?

adamhjk commented 8 years ago

I understand the proposal. :) I think part 2 is sufficiently dealt with by every file in a directory being processed as a template, and binary files can be dealt with by simply copying them instead of processing them and printing a warning. It means I can start from a copy of an existing config file without worrying about mapping the file extension back and forth, or messing up when I copy it. How many times have you forgotten the extension in Chef, or mis-spelled it, and wound up with wrong values, or run time errors?

If everyones thumbs up, I'm fine, but I think its a step backwards myself.

freethejazz commented 8 years ago

While the initial question is "would adding extensions help avoid confusion", I think it would also be useful to look at the change from a few differents perspectives: technology, usability, and learnability. When I say usability here, I mean "the user experience given that you've already learned how to do the new thing". By learnability, I mean "the ease or lack of ease around picking up the new thing."

The immediate technological impact, other than the implications of a breaking change which I'm not going to address, would be that explicitly specifying an extension would resolve #1120. However, other technological solutions and proposals exist to solve that problem. Resolving #1120 would be fringe benefit of adding an extension for templates, but would be weak argument by itself. For that reason, I think it'd be good to decouple that potential solution from the decision.

Regarding usability, I think the existing solution is a lot more friendly from an interaction point of view. While adding a specific extension certainly adds an explicit signal that some interpolation will happen, I don't know that adding explicit signals is always the best UX.

For example, when having a conversation, I'll often ask questions but will rarely say "I'm about to ask you a question" for the purpose of clarifying what I'm doing. The fact that I'm asking a question is implicit in the word choice and intonation of what I'm saying. The fact that I use a handlebars expression in a file should be enough for the system to know I'm trying to do. I would rather be able to just start adding handlebars expressions to a file and have Habitat do the right thing. With the proposed solution, I not only have to add the handlebars expression (which is for me), but I also have to add the handlebars extension (which is for the system). I think the usability of the existing solution does more to meet the user (vs the user having to meet the system), and therefore it's friendlier and easier to use.

Learnability is hard to quantify since it'll be different for people with different backgrounds and technical proficiencies. One way might be to list the number of statements required to understand how to use the feature.

Current approach:

  1. "It's ok to use handlebars expressions in any file within the hooks and config dirs."

Approach with extensions:

  1. "Any file with an .hbs extension will be interpolated and rendered, as long as it's within the hooks or config dirs"
  2. "Any scripts referencing a templatized file should actually refer to the yet-to-exist version of that file with the .hbs extension dropped"
  3. "If I want to start interpolating config values in an existing file I need to rename it".

It could be argued that individuals with prior knowledge of how templating works will be able to intuit 2 and 3 above, and part of number one. That leaves them with one rule: templates have to be within the hooks or config dirs. But that one rule is the same rule you'd have to learn with the current solution anyway. So, at best, the proposed solution can be described as succinctly as the existing solution but only given that the user has a certain prior knowledge.

I'm all for maximizing user experience, pushing the complexity deeper into the system, and reducing the amount of prior knowledge needed to enjoy using a system, so I'd support keeping the existing approach.

reset commented 8 years ago

I am :+1: because I'm not sure leaving the extension off of files adds to a developer's experience. We had this discussion many months prior when we renamed Bldrfile to plan.sh. The benefits aren't huge and there isn't a large enumeration of them, but with a file extension:

@adamhjk I'm not a hard yes or no, but I did want to see what you thought after framing this conversation the same way we had when we chatted about adding the extension to our plan entry point (plan.sh)

adamhjk commented 8 years ago

I really think the UX case is strong for leaving it off. The technical problem needs to be solved simply by having binary files not be processed, and answer the question of what we do with those files (likely just copy them.)

I don't think it's better from the initial presentation. 99% of the time if you're porting existing software, you are copying a config file, then templatizing it. How often in Chef have you forgotten to add .erb? And then been confused because it never rendered? Or you get a runtime exception? This is the same thing.

Syntax highlighting is interesting, but it won't stack - for example, you might gain handlebars highlighting, but you'll loose Apache highlighting.

tevert commented 8 years ago

I :thumbsup:'d because I think ".hbs" requires less explanation than the fact that everything in the config dir will be processed. My instincts trend towards explicit file extensions; I've never had the issue with forgetting .erb.

That being said - the workflow I've used in the past may not make as much sense here. I'm used to writing up chef/habitat from nothing and then pulling in config details from outside. When I do that, I'm thinking "I'm writing a template", and I pop the extension on. In @adamhjk's scenario, where you're starting from the existing file and just templating a couple of things, it's probably much more common to forget that.

freethejazz commented 7 years ago

I don't think .hbs requires less explanation because no matter what you still have to tell users that they can only use .hbs in the config and hooks dirs. Without that explanation you might have users do something like:

├── config
│   └── application.properties.hbs
├── default.toml
├── someotherfile.patch.hbs
├── hooks
│   ├── init
│   └── run
└── plan.sh

I haven't looked into the source enough, but based on the documentation, someotherfile.patch.hbs will not be interpolated. So as a new user to the system, even though I know what .hbs means in terms of what I can expect regarding contents of the file and processing, I have no way of knowing where I can or cannot use template files.

Another thing to think about is what the most common behavior will be. I think for the config directory, the most common behavior will be to interpolate values by a vast majority. My thinking is that if you didn't want to be able to interpolate the values, then you might as well hard-code those values somewhere else. There are organizational benefits of the config dir that might result in some files not being templates, but I'd argue that would be the minority. For the hooks directory, it's possible it could go either way, but every single init and run script in the core-plans repository uses interpolation.

So given that the default usage, and possibly almost exclusive usage, of the hooks and config dirs is to use templates I'd question whether it make sense to require the user to add an extension for all of the files. Should we be taxing the user that heavily (having to rebuild a package), for forgetting to put an extension that signifies something that we can safely assume based on it's context?

juliandunn commented 7 years ago

After some discussion with the team, we've decided we are not going to add this feature to Habitat. We feel it is sufficient that users assume all files inside config are templates. Thank you everyone for your input.