A suggestion for autocreation of modman file
Opened this issue · 26 comments
Hi Colin,
This is a suggestion, not an issue. How about an autocreate for the modman file? This would be very useful for when people retroactively create a modman file for their module rather than writing one when developing, maybe because it is a third-party module rather than their own.
How it would work
-bash$ modman automodman- the script would traverse the directories below the one it was called from
- at each directory level it would print the path and ask
include 'folder/path', [y] [n] - if the answer was no but the folder contained files it would then loop through the files asking
include folder/path/filename [y] [n] - If the answer was yes to either of the above it would create a simple modman line where both left and right path matched (I think that if users are going have a different folder structure in their modman folder to that in Magento this would still save them half the work of creating a modman file.)
- it would proceed to the next directory, either below its current level, or at a sibling level, or by returning to the parent and traversing down to the next in sequence. At each level the process 1 to 4 would be repeated
- control C would break operation
What do you think?
If you think it would be useful perhaps we could collaborate?
- it could detect any conflicts with existing in Magento dirs/symlinks
- it should ask
include 'folder/path', [Y] [n]andYis default option (just hit enter)
interesting, the [Y] [n] point is obviously straightforward.
Your first point assumes that you would running modman automodman from a Mage site. Presently a Modman checkout requires that a modman file already exists, do you foresee that you would use svn checkout instead?
Personally I was assuming that users would be running modman automodman in a folder tree outside of a Mage site before their first commit - though I think that this alternative workflow would avoid conflicts and therefore be very nice.
thinking about it, it could be run in two modes,
- as a command inside a folder lacking a modman file that was contained within the basedir of the modman deploy root, (how that folder got placed there would be up to the developer probably as the result of a move or copy command).
- or as an
--automodmanoption ofmodman checkout, if the option was present modman would not require a modman file and would instead use the process to create one.
For safety I don't think that it should create the symlinks unless the--forceoption was used, instead it should break after the modman file is created to allow the developer to check the modman file.
I'm thinking of creating a UML activity diagram to show my proposed flow, which I'll post that back here incase anyone wants to comment.
Here is the Activity Diagram for modman automodman
w00t :D
Here is the Activity Diagram for modman checkout modulename --automodman [--force]
I'm going to start work on coding this and will and will do a pull request when I'm finished
erm... --force always seems to be true for a checkout as its being set on line 870
Right use my own --autoforce option
Logic Structure as comments prior to coding it
# returns 0 on success 1 on error
# given path by option?
# no
#where are we?
#project root
#error
#base directory
#error
#a module folder
#set module
# yes
#set module
#does the modman file exist?
#yes
#echo file exists and exit without error
#no
#touch modman
#loop through files (banned include modman and hidden files)
#pre-existing path as part of ananother module
#yes
#error
#no
#include path yes no
#yes
#write path to modman
#was it a directory or file
#file - continue
#directory - stop modman recursing
#no
#continue
#no more paths
#return success
On line 349 in function apply_modman_file
# Assume target == real if only one path is given
if [ -z "$real" ]; then
real="$target"
fi
This seems to suggest that only one half of the path is needed - though I can't find this documented on the wiki
Think I'll create both sides anyway as I think this is what most people will expect to see
I'm changing the [Y][n] default to [N][y] - I've found that when traversing down a tree, say:
app/code/community/module
You say n three times before you say Y on the last folder, therefore N is the more common option
testing if the pathname exists in another modman file doesn't test if a parent directory exists in another modman - I'll think about that over the weekend
I've worked it out - automodman now tests if parent exists and tells you
A path was found that exists in below modman description file
/var/www/magento/.modman/sweetooth/modman:app/code/community/TBT app/code/community/TBT
It is therefore presently illegal for it to be included in this module.
press enter to continue...
As you can see above anyone wanting to try out this code its now on pull request #69
Nice. Maybe you add a "simple" mode, where it just adds every file it finds. I am doing find . -type f > modman (inside the module) as a first step right now …
@bobbyburden there are indeed many ways to remove the proverbial four legged feline mammal from its epidermis 😉
@joh-klein could be useful - I also considered the filename* glob pattern as way of achieving the adding of every file, and I'll probably get round to adding it to my fork the next time it's useful to me...
ie
include '/this/is/your/path/': [N][y][*]
which would create
/this/is/your/path/* /this/is/your/path/*
@waynetheisinger First of all, I am very impressed with your thoroughness. I think this is a good feature and it looks like you've implemented it really well and it doesn't break anything or fundamentally change anything. So, the only reason I haven't merged it is I haven't had time to test it out and look at it more closely, then I forgot about it.. So, sorry for that, I didn't mean to ignore your work.
I will add one thing in general is I like to stay away from building in too many features and options. For example, if a user wants to checkout/clone a repo that doesn't have a modman file, then they can already easily do this by:
$ cd .modman
$ git clone ... {module}
$ cd {module}
$ modman automodman
So I don't see the need in this case to complicate the checkout/clone command.
@colinmollenhour you're probably right about not needing to change the checkout/clone command
Also my fork has gone a little stale so if you are interested in doing a merge then I'll get mine up to date with your trunk and remove the checkout/clone code
There is also a small bug that I want to fix - it gives a false positive in the error checking for pre-existing paths in other modules, if comments in the other module, match the path that automodman is testing against - I think it should only throw the error for actual modman paths not those that are commented out....
I'll fix that too and then do another pull request
Yes, I am willing to merge this feature after said changes. Thanks!
@colinmollenhour I've made those changes and sent through another pull request... Thanks
just for the sake of completeness, here is a link to the tool already capable of generating modman files:
https://github.com/mhauri/generate-modman