8000 Use poyo for user rc files by hackebrot · Pull Request #621 · cookiecutter/cookiecutter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use poyo for user rc files #621

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 7, 2016
Merged

Use poyo for user rc files #621

merged 3 commits into from
Jan 7, 2016

Conversation

hackebrot
Copy link
Member

I wrote a YAML parser for simple files. 🐔

https://pypi.python.org/pypi/poyo

It should be fine for cookiecutterrc files and may help us overcome the troubles with PyYAML and ruamel.yaml (#557, #569). 😟

@hackebrot hackebrot added the needs-review PR Only: This PR require review from other developer label Dec 28, 2015
@codecov-io
Copy link

Current coverage is 99.63%

Merging #621 into master will increase coverage by +0.36% as of d496b04

@@            master    #621   diff @@
======================================
  Files           12      12       
  Stmts          549     546     -3
  Branches         0       0       
  Methods          0       0       
======================================
- Hit            545     544     -1
  Partial          0       0       
+ Missed           4       2     -2

Review entire Coverage Diff as of d496b04

Uncovered Suggestions

  1. +0.37% via ...ookiecutter/utils.py#27...28

Powered by Codecov. Updated on successful CI builds.

This was referenced Dec 28, 2015
Closed
Closed
@pydanny
Copy link
Member
pydanny commented Jan 6, 2016

I'm having trouble getting this PR to pass tests locally. This is an example error:

config_path = '/Users/danielgreenfeld/.cookiecutterrc'

    def get_config(config_path):
        """
        Retrieve the config from the specified path, returning it as a config dict.
        """

        if not os.path.exists(config_path):
            raise ConfigDoesNotExistException

        logger.debug('config_path is {0}'.format(config_path))
        with io.open(config_path, encoding='utf-8') as file_handle:
            try:
                yaml_dict = poyo.parse_string(file_handle.read())
            except poyo.exceptions.PoyoException as e:
                raise InvalidConfiguration(
                    'Unable to parse YAML file {}. Error: {}'
>                   ''.format(config_path, e)
                )
E               cookiecutter.exceptions.InvalidConfiguration: Unable to parse YAML file /Users/danielgreenfeld/.cookiecutterrc. Error: None of the known patterns match for default_context
E                   full_name:
E                   email: "firstname.lastname@gmail.com"
E                   github_username: "example"
E               cookiecutters_dir: "/home/example/some-path-to-templates"

cookiecutter/config.py:49: InvalidConfiguration

I'm going to attempt running tests for Poyo itself.

@pydanny
Copy link
Member
pydanny commented Jan 6, 2016

Poyo's tests pass for me, but this PR fails. I've rebuilt my virtualenv and tox environments a few times to no avail. Digging into to see where the problem is...

@pydanny
Copy link
Member
pydanny commented Jan 7, 2016

The problem was on my end with a corrupted .cookiecutterrc file. By corrupted, it had a borked line ending.

pydanny added a commit that referenced this pull request Jan 7, 2016
@pydanny
Merge pull request #621 from hackebrot/use-poyo-for-user-rc-files
a4c8e99 
Use poyo for user rc files
@pydanny pydanny merged commit a4c8e99 into cookiecutter:master Jan 7, 2016
pydanny added a commit that referenced this pull request Jan 7, 2016
@pydanny
History update for #621
f846865
@hackebrot hackebrot deleted the use-poyo-for-user-rc-files branch January 7, 2016 21:32
@vincentbernat
Copy link
Contributor

Would it be possible to support whatever YAML parser is already present on the system? I can't package (and maintain) all the YAML parsers on earth just to get cookiecutter in Debian.

@hackebrot
Copy link
Member Author

@vincentbernat I'd prefer to get rid of YAML altogether

@pydanny
Copy link
Member
pydanny commented May 21, 2016

The problem we have is that the other YAML parsers for Python aren't maintained (PyYAML) and fail in some places or are maintained (Raumel.YAML) and fail in others. The area of failure is over multiple operating systems and versions of python. Trying to gracefully allow local parsers of YAML is actually challenging.

Creative setup.py configurations are tricky to try and garantee. We know because we tried. And ran into many problems.

Poyo is pure python. It works everywhere. By breaking it into its own project we can better test it against multiple python distress and multiple flavors of Linux, OSX, and Windows.

@vincentbernat
Copy link
Contributor

In Debian, both of them work fine and the problems they have on other OS do not really concern us. With poyo, I need to introduce a new package whose description would be something like "pure Python YAML parser that doesn't really parse YAML". I will be told that such a package shouldn't be in Debian (not because it doesn't do its job but because we already have two YAML parsers for Python and it would be odd to introduce another one that only accepts a subset of YAML).

I can just patch my copy of cookiecutter to use PyYAML or ruamel.yaml if you don't want more work on this. Would this be OK?
8000

@vincentbernat
Copy link
Contributor

FYI, I applied the following patch to avoid an additional dependency: http://sources.debian.net/src/cookiecutter/1.4.0-1/debian/patches/0002-Use-PyYAML-instead-of-poyo.patch/

@hackebrot
Copy link
Member Author

Thanks for the update @vincentbernat! 🙇

@pydanny pydanny mentioned this pull request Dec 22, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs-review PR Only: This PR require review from other developer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hackebrot @codecov-io @pydanny @vincentbernat
0