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 pyproject.toml based configuration #401

Closed
petoknm opened this issue Sep 21, 2021 · 7 comments · Fixed by #473
Closed

Support pyproject.toml based configuration #401

petoknm opened this issue Sep 21, 2021 · 7 comments · Fixed by #473

Comments

@petoknm
Copy link

petoknm commented Sep 21, 2021

Is your feature request related to a problem? Please describe.
I can't use pyproject.toml file to configure rope (ignored_resources).

Describe the solution you'd like

[tool.rope]
ignored_resources = ["venv"]
@lieryan
Copy link
Member

lieryan commented Sep 21, 2021

Hi thank you for using rope. rope's configuration file is just a Python script in .ropeproject/config.py, this file is normally created when you first used rope. So if you have a couple configs that you want to read off an existing pyproject.toml, you can just do that by doing something like this in your .ropeproject/config.py:

def set_prefs(prefs):
    toml_prefs = toml.load(project_path + './pyproject.toml')['tool.rope']
    prefs.update(toml_prefs)

If we want to make this an officially supported ways to configure rope though, we'll want to think a bit more about the configuration schema in a way that makes sense to both rope and pyproject.toml; and figure out what to do to read settings that are common to multiple tools like indent sizes.

I think this looks like a pretty straightforward ticket to add this into rope/project.py or to document how to do the config.py change above. I'm open to pull request for this if you want to take to implement this ticket?

@akbhuker
Copy link

hey, can I have this issue?
I'm quite new to this so I might need some guidance

@lieryan
Copy link
Member

lieryan commented Sep 22, 2021

Thanks for the interest to look into this issue. You're welcome to take it, @akbhuker. I've assigned the issue to you.

I'd suggest to start looking in the _init_prefs() in:

rope/rope/base/project.py

Lines 256 to 275 in 4dba2ff

def _init_prefs(self, prefs):
run_globals = {}
if self.ropefolder is not None:
config = self.get_file(self.ropefolder.path + '/config.py')
run_globals.update({'__name__': '__main__',
'__builtins__': __builtins__,
'__file__': config.real_path})
if config.exists():
config = self.ropefolder.get_child('config.py')
pycompat.execfile(config.real_path, run_globals)
else:
exec(self._default_config(), run_globals)
if 'set_prefs' in run_globals:
run_globals['set_prefs'](self.prefs)
for key, value in prefs.items():
self.prefs[key] = value
self._init_other_parts()
self._init_ropefolder()
if 'project_opened' in run_globals:
run_globals['project_opened'](self)

Let me know if you need any thing else on the issue.

@akbhuker akbhuker removed their assignment Jan 4, 2022
@lieryan lieryan added this to the 0.25.0 milestone Apr 1, 2022
@bagel897
Copy link
Contributor

bagel897 commented May 10, 2022

I will write a PR to fix this issue.
I've been working on a library for this kind of problem.
The tool (rope) would define the configuration options in a pydantic model, and pytoolconfig will fill in the values from the appropriate file.
Additionally, it will detect the minimum python version, making it much easier for ast to target projects supporting older python versions than rope.
Alternatively, I can directly read from the pyproject.toml file.
Regardless, the best option is to use tomli which requires python > 3.7. It will be included in the standard library in 3.11. We could pin it to an old version but I feel like that would be bad practice.
@lieryan What is your opinion?

@Thibi2000
Copy link

Thibi2000 commented May 11, 2022

Not entirely related but is there any future plan to use setup.cfg files?

A flow could be for rope to read the setup.cfg file and generate a .ropeproject/config.py file based on the settings defined in setup.cfg? this way only parsing the setup.cfg file has to be added, or would this be a bad idea?

I am willing to add this if it is wanted.

@bagel897
Copy link
Contributor

pyproject.toml is a standard. I can add setup.cfg support (especially if using the library I'm working on), but I don't know if its a good idea.

@lieryan
Copy link
Member

lieryan commented May 12, 2022

We can specify in setup.py to install tomllib/tomli when using older Python version, but otherwise prefer the standard library version.

Rope should avoid pinning its dependencies to a particular version, as to minimise version conflict with application dependencies.

Personally, I don't favour a solution that generates .ropeproject/config.py from setup.cfg/pyproject.toml. My preference is for the default .ropeproject/config.py to include a clause to read config from setup.cfg and pyproject.toml if they exist. That way, .ropeproject/config.py and the declarative config can coexist, users can start with using pyproject.toml/setup.cfg defined as project-wide, but then override them for personal tweaks/preference in config.py.

Perhaps, in the future, rope can avoid automatically generating .ropeproject/config.py file and instead to just use the default config.py if the file doesn't exist, which can then just read from setup.cfg and/or pyproject.toml if they exist. That way, users that don't need the full power of .ropeproject/config.py can just define all their settings in pyproject.toml or setup.cfg file instead and not be burdened with an additional .ropeproject/config.py.

While there may be some potential security issues that will need to be considered with .ropeproject/config.py, I think it's quite useful that .ropeproject/config.py can contain executable code.

I'll accept any PRs that implements these alternative config parsers, whether that's just pyproject.toml, setup.cfg, or both.

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

Successfully merging a pull request may close this issue.

5 participants