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.

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

Support different names for base and non-base environment #180

Merged
merged 33 commits into from
May 8, 2024
Merged

Support different names for base and non-base environment #180

Show file tree
Hide file tree
Changes from 17 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
36 changes: 36 additions & 0 deletions docs/source/defining-shortcuts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,42 @@ The full list of available placeholders is available at {ref}`placeholders`.
This is not using any customization options or advanced features. It's the bare minimum to make it
work: a name, the command, and the target platforms.

## Specifying different shortcut names for base and non-base environments

If environments are supported, different naming schemes can be specified for installations into
the base environment and non-base environments.
To do this, the `name` property must be a dictionary with the keys `target_environment_is_base`
and `target_environment_is_not_base` for installations into the base and non-base environment,
respectively.


The example below creates a shortcut called with the name "Launch Turtle" if installed into the
base environment. If installed into an environment called, e.g., `turtle`, the name of the shortcut
is "Launch Turtle (turtle)". This was the default behavior of `menuinst` version 1.

```json
{
"$schema": "https://json-schema.org/draft-07/schema",
"$id": "https://schemas.conda.io/menuinst-1.schema.json",
"menu_name": "Python {{ PY_VER }}",
"menu_items": [
{
"name": {
"target_environment_is_base": "Launch Turtle",
"target_environment_is_not_base": "Launch Turtle ({{ ENV_NAME }})"
Comment on lines +79 to +80
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking of non-conda implementations of the schema (where there's no "base" environment, like pixi or micromamba), and whether they'll have to deal with these semantics. They'll probably ignore *_is_base, but will have to handle *_is_not_base.

In a way, it looks to me that target_environment_is_not_base is the default case, while target_environment_is_base seems to be a special case in the grand scheme of things.

Should we have this as default and target_environment_is_base, instead? I know I'm bikeshedding a bit, but I wanted to make sure that other angles are considered.

cc @wolfv for awareness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think default is ambiguous and what's the most common use case for micromamba and pixi is not necessarily the use case for others. I would say that for conda, the base environment case is much more common since the console shortcut, e.g., is installed into the base environment via the installer.

I think explicit is better than implicit.

}
"command": ["python", "-m", "turtle"],
"activate": true,
"platforms": {
"linux": {},
"osx": {},
"win": {}
}
}
]
}
```

## Associate your shortcut with file types and URL protocols

### File types
Expand Down
28 changes: 24 additions & 4 deletions menuinst/_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,19 @@ class BasePlatformSpecific(BaseModel):
* Default value is always ``None``.
"""

name: Optional[constr(min_length=1)] = None
"The name of the menu item"
name: Optional[
Union[
constr(min_length=1),
Dict[constr(regex=r"^target_environment_is(_not)?_base$"), constr(min_length=1)],
marcoesters marked this conversation as resolved.
Show resolved Hide resolved
]
] = None
"""
The name of the menu item

Can be a dictionary to use different names for installations into the base
or a non-base environment using the keys `target_environment_is_base` and
`target_environment_is_not_base`, respectively.
"""
description: Optional[str] = None
"A longer description of the menu item. Shown on popup messages."
icon: Optional[constr(min_length=1)] = None
Expand Down Expand Up @@ -340,8 +351,17 @@ class Platforms(BaseModel):
class MenuItem(BaseModel):
"Instructions to create a menu item across operating systems."

name: constr(min_length=1) = ...
"The name of the menu item."
name: Union[
constr(min_length=1),
Dict[constr(regex=r"^target_environment_is_(not_)?base$"), constr(min_length=1)],
marcoesters marked this conversation as resolved.
Show resolved Hide resolved
] = ...
"""
The name of the menu item.

Can be a dictionary to use different names for installations into the base
or a non-base environment using the keys `target_environment_is_base` and
`target_environment_is_not_base`, respectively.
"""
description: str = ...
"A longer description of the menu item. Shown on popup messages."
command: conlist(str, min_items=1) = ...
Expand Down
84 changes: 76 additions & 8 deletions menuinst/data/menuinst.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,25 @@
"properties": {
"name": {
"title": "Name",
"minLength": 1,
"type": "string"
"anyOf": [
{
"type": "string",
"minLength": 1
},
{
"type": "object",
"patternProperties": {
"^target_environment_is(_not)?_base$": {
"type": "string",
"minLength": 1
}
},
"additionalProperties": {
"type": "string",
"minLength": 1
}
}
]
},
"description": {
"title": "Description",
Expand Down Expand Up @@ -359,8 +376,25 @@
"properties": {
"name": {
"title": "Name",
"minLength": 1,
"type": "string"
"anyOf": [
{
"type": "string",
"minLength": 1
},
{
"type": "object",
"patternProperties": {
"^target_environment_is(_not)?_base$": {
"type": "string",
"minLength": 1
}
},
"additionalProperties": {
"type": "string",
"minLength": 1
}
}
]
},
"description": {
"title": "Description",
Expand Down Expand Up @@ -513,8 +547,25 @@
"properties": {
"name": {
"title": "Name",
"minLength": 1,
"type": "string"
"anyOf": [
{
"type": "string",
"minLength": 1
},
{
"type": "object",
"patternProperties": {
"^target_environment_is(_not)?_base$": {
"type": "string",
"minLength": 1
}
},
"additionalProperties": {
"type": "string",
"minLength": 1
}
}
]
},
"description": {
"title": "Description",
Expand Down Expand Up @@ -615,8 +666,25 @@
"properties": {
"name": {
"title": "Name",
"minLength": 1,
"type": "string"
"anyOf": [
{
"type": "string",
"minLength": 1
},
{
"type": "object",
"patternProperties": {
"^target_environment_is_(not_)?base$": {
"type": "string",
"minLength": 1
}
},
"additionalProperties": {
"type": "string",
"minLength": 1
}
}
]
},
"description": {
"title": "Description",
Expand Down
13 changes: 12 additions & 1 deletion menuinst/platforms/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ def __init__(
self.prefix = Path(prefix)
self.base_prefix = Path(base_prefix)

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

def create(self) -> List[Path]:
raise NotImplementedError
Expand Down Expand Up @@ -137,6 +140,14 @@ def __init__(self, menu: Menu, metadata: dict):
self.menu = menu
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:
name = self.metadata["name"].get("target_environment_is_base", "")
marcoesters marked this conversation as resolved.
Show resolved Hide resolved
if not name:
raise KeyError("Cannot parse `name` from dictionary representation.")
marcoesters marked this conversation as resolved.
Show resolved Hide resolved
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
def location(self) -> Path:
Expand Down
3 changes: 1 addition & 2 deletions menuinst/platforms/win.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,8 @@ def _paths(self) -> Tuple[Path, ...]:
return tuple(paths)

def _shortcut_filename(self, ext: str = "lnk"):
env_suffix = f" ({self.menu.env_name})" if self.menu.env_name else ""
ext = f".{ext}" if ext else ""
return f"{self.render_key('name', extra={})}{env_suffix}{ext}"
return f"{self.render_key('name', extra={})}{ext}"

def _path_for_script(self) -> Path:
return Path(self.menu.placeholders["MENU_DIR"]) / self._shortcut_filename("bat")
Expand Down
19 changes: 19 additions & 0 deletions news/180-support-different-names-outside-base
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
### Enhancements

* Support different name for shortcuts within and outside base environment (support v1 behavior)
marcoesters marked this conversation as resolved.
Show resolved Hide resolved

### Bug fixes

* <news item>

### Deprecations

* <news item>

### Docs

* <news item>

### Other

* <news item>
38 changes: 38 additions & 0 deletions tests/data/jsons/menu-name.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"$schema": "https://json-schema.org/draft-07/schema",
"$id": "https://schemas.conda.io/menuinst-1.schema.json",
"menu_name": "Package",
"menu_items": [
{
"name": {
"target_environment_is_base": "A",
"target_environment_is_not_base": "A_not_in_base"
},
"description": "This will echo environment variables for test purposes",
"icon": null,
"command": [
"echo",
"A"
],
"platforms": {
"win": {},
"linux": {},
"osx": {}
}
},
{
"name": "B",
"description": "This will echo environment variables for test purposes",
"icon": null,
"command": [
"echo",
"B"
],
"platforms": {
"win": {},
"linux": {},
"osx": {}
}
}
]
}
29 changes: 29 additions & 0 deletions tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,3 +273,32 @@ def test_url_protocol_association(delete_files):
url_to_open=url,
expected_output=url,
)


@pytest.mark.parametrize("target_env_is_base", (True, False))
def test_name_dictionary(target_env_is_base):
tmp_base_path = mkdtemp()
tmp_target_path = tmp_base_path if target_env_is_base else mkdtemp()
(Path(tmp_base_path) / ".nonadmin").touch()
if not target_env_is_base:
(Path(tmp_target_path) / ".nonadmin").touch()
abs_json_path = DATA / "jsons" / "menu-name.json"
menu_items = install(abs_json_path, target_prefix=tmp_target_path, base_prefix=tmp_base_path)
try:
if PLATFORM == "linux":
expected = {
"package_a" if target_env_is_base else "package_a-not-in-base",
"package_b",
"package",
}
else:
expected = {
"A" if target_env_is_base else "A_not_in_base",
"B",
}
if PLATFORM == "win":
expected.update(["Package"])
item_names = {item.stem for item in menu_items}
assert item_names == expected
finally:
remove(abs_json_path, target_prefix=tmp_target_path, base_prefix=tmp_base_path)