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

Support nested folder hierarchies for projections #14

Closed
glittershark opened this issue Apr 11, 2014 · 60 comments
Closed

Support nested folder hierarchies for projections #14

glittershark opened this issue Apr 11, 2014 · 60 comments

Comments

@glittershark
Copy link

Some tools like Zend Framework have a folder hierarchy something like the following:

application/
|   modules/
|   |   mod1/
|   |   |   controllers/
|   |   |   |   MyController.php
|   |   |   |   ...
|   |   mod2/
|   |   |   controllers/
|   |   |   |   MyController.php
|   |   |   |   ...
|   |   ...
|   ...

It'd be nice in projectile if I could define a projection like:

{
    "application/modules/*/controllers/*Controller.php": {
        "command": "controller"
    }
}

so I could EController mod1_my

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

I also recently realized two Clojure conventions, test/foo/bar/test/baz.clj and test/foo/bar/t_baz.clj would stand to benefit from something like this as well. I think the right solution is going to involve a {} expression in the key rather than *, but I have only the vaguest idea of how that might work.

Your proposed solution has an underscore that sprang out of nowhere.

@glittershark
Copy link
Author

There would need to be some (maybe user defined) way to separate the path components, I would think. Maybe I'm wrong about that though.

What about named regex groups for the file glob? Something like:

{
    "application/modules/(*)/controllers/(*)Controller.php": {
        "command": "controller",
        "name": "{1}_{2}"
    }
}

The named groups, and the "name" option could be optional so as to not break existing .projections.json configuration.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

Maybe if you drop the parentheses, but that's still a whole lot of added complexity for a relatively dumb problem. I'd sooner just force a join with / or smoosh them together. I'm not sure I buy why it needs to be an underscore here at any rate.

@glittershark
Copy link
Author

Simplicity is fine. I like the sound of putting a / in there

@qstrahl
Copy link

qstrahl commented Apr 11, 2014

Seconding the motion to force a join with /

On 11 April 2014 12:11, Griffin Smith notifications@github.com wrote:

Simplicity is fine. I like the sound of putting a / in there

Reply to this email directly or view it on GitHubhttps://github.com//issues/14#issuecomment-40221125
.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

Note that smooshing just means you'd have to write "application/modules/*controllers/*Controller.php", which is way more consistent and opens up some other possibilities (at the cost of possible but unlikely false positives, DWIM, and just plain looking weird).

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

Can someone confirm that both of these return the expected results?

:echo glob("application/modules/**/*controllers/**/*Controller.php")
:echo glob("application/modules/**/*/controllers/**/*Controller.php")

@glittershark
Copy link
Author

Looks right to me

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

@glittershark I don't know what you mean.

@glittershark
Copy link
Author

If you're referring to my other comment, I deleted it because I didn't know what I meant either :)

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

Was leaning towards smooshing then realized false positives for test/*t_*.clj wouldn't be all that unlikely. So I think / joining will win, but probably with a constraint there has to be a / after the first *.

@qstrahl
Copy link

qstrahl commented Apr 11, 2014

Seems sound to me. Looking forward to support for this!

@tommcdo
Copy link
Contributor

tommcdo commented Apr 11, 2014

I feel like (space) would be a good delimiter. I mean, each wildcard along the way could contain/'s and it might just be ridiculous to disambiguate. It's probably (hopefully) far less likely that there will be spaces in the filenames of a code project.

With this style, each wildcard is kind of like a command argument, each with dedicated completion. It would be nice if completion would allow wildcard positions to be skipped (auto-filled), but I can see that getting complicated.

How would multiple wildcards affect the language used for alternates and templates? Would {} be replaced with {1} (and so {underscore} with {1|underscore}, etc)? Or could some other assumptions be made?

@qstrahl
Copy link

qstrahl commented Apr 11, 2014

each wildcard along the way could contain /'s and it might just be ridiculous to disambiguate. It's probably (hopefully) far less likely that there will be spaces in the filenames of a code project.

Uh, could it? Last I checked, you can't have directory separators in file names on any sane system.

@glittershark
Copy link
Author

What about the ** glob?

@tommcdo
Copy link
Contributor

tommcdo commented Apr 11, 2014

The * gets interpreted by projectile as a recursive glob (**)

@qstrahl
Copy link

qstrahl commented Apr 11, 2014

I might not be thinking very hard, but I can't come up with an ambiguous case.

@tommcdo
Copy link
Contributor

tommcdo commented Apr 11, 2014

Let's say you have

foo/*/bar/*.baz

as your pattern. Then you have these files:

foo/a/bar/b/c.baz
foo/a/b/bar/c.baz

Using / would mean they both match a/b/c.

@qstrahl
Copy link

qstrahl commented Apr 11, 2014

There it is. Yeah, that's no good. =\

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

It gets ambiguous the second you match */dir/* against foo/dir/bar/dir/baz. It cancels out if we just combine everything together with slashes. This alone is an argument against combining with spaces or {1} or any of these other contrivances.

I'd actually like to switch from * to {}, because it would give us symmetry with the expansions and clear up people mistaking * for a file glob in one fell swoop, plus free up * to use as a more conventional glob. But I think {}/dir/{} is even more confusing in the context of my most recent proposed solution.

@tommcdo
Copy link
Contributor

tommcdo commented Apr 11, 2014

Correct me if I'm wrong, but for a case like that, is just as good as/. You'd have the following arguments with/:

foo/bar/dir/baz
foo/dir/bar/baz

versus the following with :

foo bar/dir/baz
foo/dir/bar baz

Both are cleared up in this case.

But / doesn't address the type of ambiguity I pointed out in my last comment, which arises from more than one match having the same smooshed value (e.g. a + b/c; and a/b + c).

@glittershark
Copy link
Author

I feel like there's a case to be made for splitting apart the behavior of * and ** in projectile, to get rid of these kinds of ambiguities.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

You're right. I guess the first * will be greedy, or we could maybe get more clever, go back to smooshing, and make {}dir/{} match lazily but greedily otherwise. (This is consistent with how you'd expect test/{}t_{}.clj to work. I think.) In the PHP example, what does a nested path look like?

Spaces are arguable in the title case but absolutely ridiculous in the context of my Clojure examples.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

@glittershark I'm trying to move away from the globbing syntax not double down on it.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

Though if I can't switch to {} it might be better than nothing.

@tommcdo
Copy link
Contributor

tommcdo commented Apr 11, 2014

What about creating files? How would you infer where to unsmoosh?

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

I guess greedy/lazy actually means "all of the nesting"/"no nesting".

@tommcdo
Copy link
Contributor

tommcdo commented Apr 11, 2014

Sounds kind of limiting. I'm picking up that we might be thinking of this differently. I'm thinking that each submatch would be accessible independently when it comes to alternates and templates. I don't know what the notation would look like, but a possible example is this:

"foo/{}/bar/{}.baz": {
    "command": "foo",
    "alternate": "foo/{1}/bar/{2}-test.baz",
    "template": [
        "class {2|underscore}"
    ]
}

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

I want some pattern for test/foo/bar/t_baz.clj that lets {} expand to foo/bar/baz. Numbered expansions aren't out of the big picture but for shit this basic they're a cop out.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

I'm also thinking forward to potential abstractions like "alternate": "test", which will fall flat on its face if there's not a singular canonical expansion.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

Does anyone even know how well projectile works on Windows? Looking at the code and first order of business will be making sure we're doing the right thing with slashes.

@tommcdo
Copy link
Contributor

tommcdo commented Apr 11, 2014

I think Windows doesn't care whether you use forward or backward slashes. That said, I haven't tried it.

@tpope
Copy link
Owner

tpope commented Apr 11, 2014

Yes you can pass either slash to glob but globbing is just one piece of the puzzle. We need to take it into account in equality checks for example. I've think I've been diligent about that but without any testing it's hard to say.

tpope added a commit that referenced this issue Apr 12, 2014
@tpope
Copy link
Owner

tpope commented Apr 12, 2014

After sleeping on it, I felt that given that ** actually maps to the problem pretty cleanly (Kohona notwithstanding), it would probably be best to just support a limited version of it, rather than force contrived curly brace semantics. I've pushed a prototype to the glob branch. I expect there are breakages around the empty ** case, at the very least. Let me know what you find.

We will probably also need to add an expansion like {dirname} that also includes the trailing slash. This might be a good time to revisit that "leading" idea you had, @tommcdo.

tpope added a commit that referenced this issue Apr 13, 2014
tpope added a commit that referenced this issue Apr 15, 2014
tpope added a commit that referenced this issue Apr 15, 2014
@tpope tpope closed this as completed in f4adc8b Apr 16, 2014
@tpope
Copy link
Owner

tpope commented Apr 16, 2014

Well this issue got quiet. I iterated a few times, and I'm pretty satisfied with the result.

@glittershark
Copy link
Author

This is pretty sweet. Thanks!

@monokrome
Copy link

Just wanting to provide a use case which I don't believe is covered by the current solution.

This same convention used by Kohana is used by some AngularJS projects. Off the top of my head, some other frameworks that provide this structure as convention include Django and Symfony. Many Marionette.JS projects also take this approach. I'm pretty sure that Rails projects are similar, but it's scaffolding makes this less of an issue.

An example of the project that I'm working on is quite similar to Kohana with the following structure:

src:
  - common:
    - module.coffee
    - partials:
      - index.jade
      - other.jade
    - controllers:
      - index.coffee
      - other.coffee
    - directives:
      - other.coffee
  - specific:
    - module.coffee
    - partials:
      - index.jade
      - some.jade
    - controllers:
      - index.coffee
      - some.coffee
    - directives:
      - some.coffee

It would be nice to have a more powerful way of doing this, because I would like to be able to create a new files using the template functionality from projectile. For instance, a command like :EController common index might create a file called src/common/controllers/index.coffee.

This file would ideally use the already-existing expansions from projectile to mock out the following initial template:

angular.module 'common'
  .service 'common.controllers.index', [
    '$scope'

  ].concat ($scope) ->
    angular.extend $scope, {}

The module is the name of the first glob. The service name is essentially the path from the first to last glob replacing slashes with dots. It's a fairly common thing for Angular modules to map to the filesystem in this way.

Right now, I am needing to create specific keys for every module - but it's a bit of a task because there are quite a few of them. The results are similar to this:

{
  "src/common/directives/*.coffee": {"command": "CommonDirective"},
  "src/common/services/*.coffee": {"command": "CommonService"},
  "src/common/controllers/*.coffee": {"command": "CommonService"},
  "src/*/module.coffee": {"command": "Module"}
}

Most of this functionality exists already in the project, but being able to glob and get references to each one is the missing component for these types of file structures is the single missing piece

@tpope
Copy link
Owner

tpope commented Apr 26, 2014

Links to real world projects would be helpful.

Those CamelCase commands are irking me. Makes me wonder if I should be enforcing some sort of normalization.

@monokrome
Copy link

The majority of Django apps follow this pattern, where there's a set of core files in either the project root or in a specific core module. This same concept translate to the other aforementioned frameworks to varying degrees of popularity.

Some examples:

Related articles:

The camel case commands seemed a bit odd to me, also. I felt like it was more conventional to type :ESource instead of Esource and it doesn't sacrifice an extra key stroke since E already requires shift be held.

@tpope
Copy link
Owner

tpope commented Apr 29, 2014

Having thought about it a few days, I think the next logical step on the path I've set us down is to allow multiple *s but only a single **. This should avoid ambiguity.

Not finding either of those repos or blog posts to contain a concrete example of nesting that can't be handled by ** and * (although perhaps there's something hiding in the massive Django one). So I'm still unconvinced this is anything but the lowest priority.

The official projectionist convention for commands is lowercase, squished together. This is already partially enforced by stripping out punctuation, and a future version might add a tolower() to the mix as well.

@monokrome
Copy link

This might just be a misunderstanding in how projectionist works with the ** glob, but I don't see how to reference it as a pattern (in a template or alternate, for example) or how to use it for creating new files.

Are these not supported use cases?

@tpope
Copy link
Owner

tpope commented Apr 30, 2014

The ** and * combine together. Pull them back apart with {dirname} and {basename} if you need to.

@monokrome
Copy link

Seems like that solved most of the issues, actually. Thank you for clarifying.

Seems like this is one is a working example for anyone that might be confused on a similar project:

{
  "src/**/services/*.coffee": {
    "command": "service",
    "template": [
      "angular.module '{dirname|dot}'",
      "  .service '{dirname|dot}.controllers.{basename|underscore}', [",
      "    '$rootScope'",
      "",
      "  ].concat ($rootScope) ->"
    ]
  }
}

@monokrome
Copy link

One lingering issue is that concatenating them together means that I have no context of ** vs directory element in the * glob, but I think that this template usage might be an unusual use case?

There is a common use case in Django apps where models.py gets long and moved to a models package instead, for instance.

@tpope
Copy link
Owner

tpope commented Apr 30, 2014

I don't understand the question.

@monokrome
Copy link

For instance, a user may have a file named src/common/services/twitter/tweet.coffee. Given the previously supplied example .projections.json file, you might try to create a new file based on a template:

:Eservice common/twitter/tweet

The slashes seem really awkward in this situation. It might be nice to allow spaces where globs are separated, but this is quite a digression.

This interface is ambiguous, because a user running the previous command would be attempting to define a module called common.services.twitter.tweet. However, getting this information for the template is impossible because {dirname} will be set to common/twitter and {basename} would be tweet. The template would produce common.twitter.services.tweet instead.

One common example of where this use case would occur is when there is a Django project with a package that defines models as a package instead of a single-file module.

I was specifically asking whether using templates in this way was an intended/supported use case, or if it's something that shouldn't be supported? If this is an intentionally unsupported use case then the current implementation seems to work. If it should be supported then it might make more sense to allow a more powerful use case.

@tpope
Copy link
Owner

tpope commented Apr 30, 2014

Bigger problem there is that the generated filename would be src/common/twitter/services/tweet.coffee. This is where support for multiple * would help, as the proper pattern would be "src/*/services/**/*.coffee".

Once that's fixed, adding another template expansion or two is a comparatively straightforward process.

@monokrome
Copy link

Good point. That definitely seems like the right solution as long as it's possible to reference the separate globs independently in the template!

The other concern that is potentially of interest here is whether having two globs implies an additional argument through the command. Would a user use :Eservice common twitter/tweet as their command?

@tpope
Copy link
Owner

tpope commented Apr 30, 2014

Not if I can help it.

On Tue, Apr 29, 2014 at 11:37 PM, Brandon R. Stoner <
notifications@github.com> wrote:

The other concern that is potentially of interest here is whether having
two globs implies an additional argument through the command, as well. IE,
would a user then use this command?

:Eservice common twitter/tweet


Reply to this email directly or view it on GitHubhttps://github.com//issues/14#issuecomment-41757582
.

@tommcdo
Copy link
Contributor

tommcdo commented Apr 30, 2014

I think two new transformations are in order: one that returns the first component, and one that returns all but the first. These will be necessary for cases where * precedes **.

My use case will involve a lot of modules/*/classes/controller/**/*.php-like patterns, so dirname and basename alone aren't enough.

Actually, with the two transformations I'm proposing (the biggest obstacle for which is what to call them), the **/* could be replaced with a **.

@tpope
Copy link
Owner

tpope commented Apr 30, 2014

I'd probably just repurpose head and tail. But that's small potatoes compared to enhancing the glob support. Unless a hero steps up I don't see this making it into 1.0.

@monokrome
Copy link

Just to note, I've seen projects where not having multiple ** is going to cause issues. This doesn't represent my current use case, but would have in the last project that I worked on. The case there is that there can be nested modules. For instance, these patterns should be possible in this case where I can reference the different paren groupings:

src\/([\w\/\\]+)\/(directives|controllers)\/([\w\/\\]+)\.coffee
src/module/directives/example.coffee
src/module/submodule/directives/example.coffee
src/module/directives/category/example.coffee
src/module/controllers/example.coffee
src/module/directives/category/example.coffee
src/module/submodule/directives/example.coffee

A live view of the expression matches can be seen here. Hovering over each line of text will show a tooltip of expected matches in these cases.

It seems like adding limitations such as allowing only one ** glob might be coming at a cost of flexibility. There will always be some solution where head, tail, etc is not enough in some projects - even with the ability to have multiple * groups.

glittershark added a commit to glittershark/projectionist that referenced this issue Jul 23, 2014
Handle multi-level globs by separating the passed file with `/`. See
tpope/vim-projectionist#14
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

No branches or pull requests

5 participants