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

Token discovery with defaults not working as expected #85

Closed
alecho opened this issue Feb 6, 2015 · 4 comments
Closed

Token discovery with defaults not working as expected #85

alecho opened this issue Feb 6, 2015 · 4 comments

Comments

@alecho
Copy link
Contributor

alecho commented Feb 6, 2015

I've noticed a couple things with the new token parsing. First, the installer only seems to give me one prompt even though the FileParse it finding all of the tokens it should. The second issue is that defaults can be overwritten through array_combine because of the order of the array keys. Third, when replacing tokens in files we probably need to replace by regex so that everything that matches both a token and any default value it may have will be replaced. This brings up my last issue which is that while it's possible to have the same "token" with different default values, only one default will be asked of the user now. Does anyone have any strong opinions on any of these issues?

Here's what we get after searching a file for tokens:

Array
(
    [0] => Array
        (
            [0] => {{PROJECT_TITLE}}
            [1] => {{PROJECT_REPO_URL:github.com/loadsys/app}}
            [2] => {{PROJECT_DESCRIPTION}}
            [3] => {{PROJECT_PRODUCTION_URL}}
            [4] => {{PROJECT_STAGING_URL}}
            [5] => {{PROJECT_MANAGEMENT_URL}}
            [6] => {{PROJECT_DOCUMENT_URL}}
            [7] => {{PROJECT_REPO_URL}}
            [8] => {{PROJECT_REPO_URL}}
            [9] => {{PROJECT_CLIENT_NAME}}
        )

    [1] => Array
        (
            [0] => PROJECT_TITLE
            [1] => PROJECT_REPO_URL
            [2] => PROJECT_DESCRIPTION
            [3] => PROJECT_PRODUCTION_URL
            [4] => PROJECT_STAGING_URL
            [5] => PROJECT_MANAGEMENT_URL
            [6] => PROJECT_DOCUMENT_URL
            [7] => PROJECT_REPO_URL
            [8] => PROJECT_REPO_URL
            [9] => PROJECT_CLIENT_NAME
        )

    [2] => Array
        (
            [0] =>
            [1] => github.com/loadsys/app
            [2] =>
            [3] =>
            [4] =>
            [5] =>
            [6] =>
            [7] =>
            [8] =>
            [9] =>
        )

)

After array_combine:

Array
(
    [PROJECT_TITLE] =>
    [PROJECT_REPO_URL] =>
    [PROJECT_DESCRIPTION] =>
    [PROJECT_PRODUCTION_URL] =>
    [PROJECT_STAGING_URL] =>
    [PROJECT_MANAGEMENT_URL] =>
    [PROJECT_DOCUMENT_URL] =>
    [PROJECT_CLIENT_NAME] =>
)

One feature I would like to implement is the ability to use another token's value as a default for anther token. This would add some complexity I'm not sold on plus possibly introduce circular references.

@beporter
Copy link
Contributor

beporter commented Feb 6, 2015

I'm not a fan of making the token system overly complex so I'm against nesting. We're already reaching the point where we've put more time into making this automatic than the amount of time it would take to manually set every token for every 3.x project we do over the next 5 years. We aren't helping that much more by continuing to iterate on this idea. We need to move on to more critical issues, like getting the dependent packages in shape for 3.0:

On the question of different defaults for the same token; I think they should be discouraged. We don't want the same conceptual idea to have two different explicit values. If the same token is meant to represent two different things, they should be two different tokens. So I say let the defaults get clobbered. We have tools (grep commands) to review them easily, so finding duplicate tokens with different defaults is pretty trivial now. It's also then pretty easy for us to manually ensure all identical tokens share an identical default.

@beporter
Copy link
Contributor

beporter commented Feb 6, 2015

Keep in mind that nesting in particular will preclude using regex to scan the files. Regex can't handle nesting properly. We'd have to develop a full parser. There's just no way that's worth the effort.

@beporter
Copy link
Contributor

Yeah, just confirming that token replacement is completely broken right now.

@beporter beporter added the focus label Jun 25, 2015
@beporter
Copy link
Contributor

On the topic of multiple defaults for the same token: If we do anything, we should track all the defaults and present them as options:

Token Name:
1. First default
2. Second Default
3. Third
[First default] Enter number or free-form: _

This wouldn't be hard to do by treating each key in the $defaults array as an array itself, and pushing additional matches into them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants