Take app_path as argument for hanami new

[waiting-for-dev]

There're a few benefits in doing so instead of taking the app name:

• Being a CLI command, it feels more natural passing a directory name
  than a ruby module name.
• Simpler code: we were re-inflecting the module name from the inflected
  directory name in the templates.
• More familiar for Ruby developers: it's what Rails does.
• Taking the basename and expanding the path, we allow something like hanami new ..
• It allows to create of the app within a nested directory.
• We allow for any combination of upper and lower case letter in the directory.

[cllns]

I like this feature. It makes sense to me: the flexibility of hanami new . then also the parity of Rails' behavior. And in most cases, it'll be the same as the existing behavior. Will we also need or want a --name (or similar) option to override the default logic for calculating the app name?

[jodosha]

@waiting-for-dev By changing the semantic of the first argument from app name (by default in creates an app in current dir) to app path, we need to handle it properly.

---

How to communicate that the default case is going to be hanami new bookshelf?

bookshelf is a relative path to current directory, but what if a developer interprets app_name differently? What if she/he thinks "oh it's a path app, I want it to be created in current directory"

E.g. developer is in ~/Code and thinks she/he wants to create the app in current dir: so she/he does hanami new ., but she/he doesn't how to specify the app name. What she/he wanted to do was actually hanami new bookshelf.

---

We need to error handle cases where the path isn't writable.

Or it's a writable, but it doesn't make sense.
Example: hanami new /, how do we infer the app name?

---

We need to normalize the path before to pass it to the generators.
This PR changes the semantic from app name to app path, but it expects a single name (bookshelf), not a path.

If we pass a dot that indicates the current path (.), we're going to generate lib/./types.rb.
If we pass a path (e.g. ~/Code/bookshelf), we're attempting to create lib/~/Code/bookshelf/types.rb.

---

Generally speaking I don't like to change the semantic of that argument, because it complicates the internal implementation.

The sense that I see for that app name argument is the following.
This argument generates an application in a subdirectory of the current dir. The directory is the underscored version of the name, the Ruby module is the camelized version.

There is a special case ., that it generates the app in the current directory, using the name of the current directory to generate the Ruby module of the app.

[waiting-for-dev]

Hey, @jodosha, thanks for looking into it.

I wanted to follow up in a second PR, but I've added the remaining work here to make my point. Please look at it and feel free to close the PR if it's not how you envision the hanami new command.

How to communicate that the default case is going to be hanami new bookshelf?

bookshelf is a relative path to current directory, but what if a developer interprets app_name differently? What if she/he thinks "oh it's a path app, I want it to be created in current directory"

E.g. developer is in ~/Code and thinks she/he wants to create the app in current dir: so she/he does hanami new ., but she/he doesn't how to specify the app name. What she/he wanted to do was actually hanami new bookshelf.

I'm not sure I follow here. Precisely, thinking that it's a path app, it makes evident that "." means the current directory. Of course, reading the docs or the output of hanami new --help is needed to understand the semantics. But that's true in either case.

We need to error handle cases where the path isn't writable.

Or it's a writable, but it doesn't make sense.
Example: hanami new /, how do we infer the app name?

"/" is a good point. We're now raising an exception in that edge case. As @cllns said, we could add an optional --name param to override what it's taken from the basename and not raise when given.

About writable, we can have better error handling, but probably the system error is informative enough. However, that can also happen if the argument is a module name.

We need to normalize the path before to pass it to the generators.
This PR changes the semantic from app name to app path, but it expects a single name (bookshelf), not a path.

That's no longer a problem as we're now taking the basename.

If we pass a dot that indicates the current path (.), we're going to generate lib/./types.rb.
If we pass a path (e.g. /Code/bookshelf), we're attempting to create lib//Code/bookshelf/types.rb.

We're now expanding the path, so that's no longer a problem. I've tested it manually, and it works. However, I cannot reproduce it on a test, as it looks like the context of "." is not taken when running the code within fs.chdir(app_path). I didn't have the time to look into it in depth, though.

IMHO, that's the most straightforward implementation, as the argument is always a path and not sometimes a module name and others a path (when it's "."). It's also way more flexible, as it allows creating apps in any directory and having them with any combination of upper and lower cases.

By the way, when testing the command, the generator went forward, but I got some errors. I think they're not related; instead, they have something to do with the recent reload changes. Also, it looks like ".." doesn't work as expected, and it rather takes one directory higher. Not sure if that's a Ruby or "dry-files" issue.

Please, let me know your thoughts 🙂

[cllns]

I think this is a case where we should follow the convention that Rails set. For what it's worth, Rails doesn't have a special error for rails new /

% rails new /
/Users/sean/.gem/ruby/3.0.3/gems/railties-7.0.0/lib/rails/generators/app_name.rb:44:in `const_defined?': wrong constant name  (NameError)

Although, to be fair, rails new . only uses the current name of the directory twice: in config/application.rb to define a module of that name, and in the default layout (which isn't very important). In Hanami, we leverage Ruby namespaces a lot, so it's a more consequential decision, since it names many files, and will be used for most generated files.

[timriley]

I agree that our hanami new should be flexible enough to be able to generate applications into explicitly specified directories, rather than just the current working directory.

In my mind, I think the simplest (and easiest to communicate) way of doing this is to:

1. Change the leading argument to app_path
2. But also accept an --app-name argument, which defaults to an underscored version of the last directory segment of the expanded app_path (so "." expands to the full path in order to determine this).

In this PR, @waiting-for-dev Has done (1) already, as well as half of (2). We just need to take things one small step further, to turn the app name into a real command argument so users can choose to provide explicit app names that diverge from their intended target directory.

I've actually wanted this behavior myself in the past, when I was generating apps for which I wanted a long, descriptive (and dasheriszed) project directory name, something that would match the intended GitHub repo name, but a more succinct name to use for the app's actual Ruby namespace name.

The way to do this now is to generate the app into a directory name matching the app's Ruby namespace name, and then rename that directory afterwards. Granted, this is easy enough to do, but it feels like you're having to work against the tools in doing so.

I think @cllns is astute in pointing out here that the name of Hanami applications is much more significant than e.g. Rails, so given this, I think it makes sense to promote the app name to a first class argument here.

If we do this, I think the resulting behaviour would be intuitive to users. In most cases the app name would not need to be specified, but if a user ever generates an app and realises the automatically determined app name isn't to their liking, then a quick check of the --help will then make it clear the extra --app-name argument is there to help them achieve their goals. (Another instance of "progressive disclosure" at play!)

[timriley]

So in terms of this PR, I'd like to see the --app-name option added as a way of completing the new feature.

[timriley]

The way we phrase this special case seems undesirable to me.

Like, if I am root on a machine and I want my hanami app to live at /, why shouldn't I be able to put it there?

Now, I think the actual problem here is that we can't determine an app name if the given app_path has no actual word as its basename.

This IMO is actually another reason to add --app-name as an option. Once we do that, we can move the location of this error to be the part of our code that determines a default app name from the given app_path if there is no explicit --app-name option provided. From there, the error could read something more like this:

Could not determine an app name from the given app_path "/". Please provide an app name with the --app-name option

Which would make much more sense to me — the problem isn't that we can't write to "/", but it's that we can't work out an app name to use given the path provided. This kind of error message would better reflect what's going on, and would also be a useful way to help our users discover this extra option in the case where they're clearly trying to do something unusual.

[waiting-for-dev]

Thanks, @timriley; I think adding --app_name as part of this PR makes sense. I'll wait for @jodosha's confirmation to be sure he's ok with taking this direction.

[jodosha]

@waiting-for-dev Thanks for this attempt, but in the end we decided to keep out of 2.0.