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 different names for base and non-base environment #180

Merged
merged 33 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b9e7cfa
Set env_name when not in base environment
marcoesters Jan 25, 2024
1f9bbd4
Add note that ENV_NAME will be added to shortcuts outside base enviro…
marcoesters Jan 25, 2024
03b40fd
Specify that env_name only gets added on Windows
marcoesters Jan 25, 2024
5f0e051
Add news
marcoesters Jan 25, 2024
42e769a
Remove one set of backtics
marcoesters Jan 25, 2024
7272333
Escape curly braces
marcoesters Jan 25, 2024
2af02fb
Add PR number to news
marcoesters Jan 26, 2024
0e7113a
Do not change Windows shortcut name
marcoesters Mar 25, 2024
4a7be20
Support different names for base and non-base environments
marcoesters Mar 27, 2024
6f7300b
Change news item to new scope
marcoesters Mar 27, 2024
d3d6d9c
Add tests for supporting different names for base and non-base enviro…
marcoesters Mar 27, 2024
3d0e36e
Make dict typing python 3.8 compatible
marcoesters Mar 27, 2024
7b83395
Simplify shortcuts for tests
marcoesters Mar 27, 2024
c223f5f
Fix Linux shortcut names in test
marcoesters Mar 27, 2024
8f39521
Do not allow dicts for menu_name
marcoesters Mar 27, 2024
422dbb8
Add missing platforms to menu-name.json
marcoesters Mar 27, 2024
1de5f43
Update docs/source/defining-shortcuts.md
marcoesters Apr 11, 2024
1909ef0
Apply suggestions from code review
marcoesters Apr 23, 2024
46e7795
Make target_environment keywords literals
marcoesters Apr 23, 2024
932f80c
Use resolve instead of absolute for prefix comparison
marcoesters Apr 23, 2024
c6730eb
Use Path.samefile instead of absolute or resolve
marcoesters Apr 23, 2024
ca9f608
Use TypedDict to represent target_env keys in schema
marcoesters Apr 23, 2024
dce5666
Use new model instead of TypedDict for easy python 3.8 compatibility
marcoesters Apr 26, 2024
bc92af7
Update documentation
marcoesters Apr 26, 2024
1d5f1be
Apply suggestions from code review [skip ci]
marcoesters Apr 29, 2024
ca52e2d
Use self.env_name as ENV_NAME placeholder
marcoesters Apr 29, 2024
9b87330
Use menu prefix
marcoesters Apr 29, 2024
0ab1e24
Use menu base_prefix
marcoesters Apr 29, 2024
8516d46
Merge branch 'main' into env-name-shortcut
marcoesters May 1, 2024
fef15d0
Remove unnecessary sys import
marcoesters May 1, 2024
2249a94
Merge branch 'env-name-shortcut' of github.com:marcoesters/menuinst i…
marcoesters May 1, 2024
9588b89
Set enviornment name to base if prefix equals base_prefix
marcoesters May 3, 2024
f186e8c
Remove or-clause for ENV_NAME placeholder
marcoesters May 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion menuinst/_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
# flake8: noqa

import json
import sys
from logging import getLogger
from pathlib import Path
from pprint import pprint
Expand Down
12 changes: 6 additions & 6 deletions menuinst/platforms/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def __init__(
self.base_prefix = Path(base_prefix)

if self.prefix.samefile(self.base_prefix):
self.env_name = None
self.env_name = "base"
else:
self.env_name = self.prefix.name
marcoesters marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -75,7 +75,7 @@ def placeholders(self) -> Dict[str, str]:
"DISTRIBUTION_NAME": self.base_prefix.name,
"BASE_PYTHON": str(self.base_prefix / "bin" / "python"),
"PREFIX": str(self.prefix),
"ENV_NAME": self.prefix.name,
"ENV_NAME": self.env_name or "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be false-y? 🤔 I think we don't need the or "" part, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I look at it again, I don't think so. I guess I was overly cautious here.

"PYTHON": str(self.prefix / "bin" / "python"),
"MENU_DIR": str(self.prefix / "Menu"),
"BIN_DIR": str(self.prefix / "bin"),
Expand Down Expand Up @@ -142,12 +142,12 @@ def __init__(self, menu: Menu, metadata: dict):
self._data = self._initialize_on_defaults(metadata)
self.metadata = self._flatten_for_platform(self._data)
if isinstance(self.metadata["name"], dict):
if self.menu.env_name:
name = self.metadata["name"].get("target_environment_is_not_base", "")
else:
if self.menu.prefix.samefile(self.menu.base_prefix):
name = self.metadata["name"].get("target_environment_is_base", "")
else:
name = self.metadata["name"].get("target_environment_is_not_base", "")
if not name:
raise KeyError("Cannot parse `name` from dictionary representation.")
raise ValueError("Cannot parse `name` from dictionary representation.")
self.metadata["name"] = name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point we have not rendered the {{ variables }}, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct.

The tests for a dictionary representation of the name would have failed by then, so even if code changes render the name at that point, the tests will catch that.


@property
Expand Down