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

DRAFT: Order based on filenames or partial path #52

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

DRAFT: Order based on filenames or partial path #52

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions pytest_order/sorter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,21 @@ def items_from_class_label(self, label: str, item: Item) -> List[Item]:
items.append(self.node_ids[node_id])
return items

def items_from_path_label(self, label: str, item: Item) -> List[Item]:
items = []
item_id = item.node_id
label_len = len(label)
for node_id in self.node_ids:
if node_id.count("::") == 2:
path_index = node_id.index("::")
if label.endswith('/') and label in node_id: # directory ordering
Copy link
Member

Choose a reason for hiding this comment

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

The in check here could be ambiguous - maybe node_id.startswith(label)? Or we need more checks here...

Copy link
Author

@Joacchim Joacchim Aug 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I took the approach of checking endswith() "in" in order to match any part of a directory's path, even if not fully specified (as it allows shorter and more concise specification in the decorator parameters).

Copy link
Member

Choose a reason for hiding this comment

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

Yes, actually I did the same with endswith, which wouldn't work in your case, of course... given that it needs to end with a slash this may be ok (it would theoretically allow ambiguities, but the user would be responsible to avoid them).

items.append(self.node_ids[node_id])
elif node_id[:path_index].endswith(label): # end-of-filename ordering
Copy link
Member

Choose a reason for hiding this comment

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

This implies that there is only one directory ending with label, though it could be documented that label has to be unambiguous in that respect.

Copy link
Author

Choose a reason for hiding this comment

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

I thought it would be easier to require ending the label with a / to refer to a directory, which would incidentally avoid confusing a directory and any other kind of path/entity.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, makes sense.

id_start = node_id[:path_index - label_len]
if item_id.startswith(id_start):
items.append(self.node_ids[node_id])
return items

def handle_before_or_after_mark(
self, item: Item, mark: Mark, marker_name: str, is_after: bool
) -> bool:
Expand All @@ -247,9 +262,19 @@ def is_mark_for_class() -> bool:
self.rel_marks.insert(0, rel_mark)
item.inc_rel_marks()
return True
else:
if is_mark_for_class():
items = self.items_from_class_label(marker_name, item)
elif is_mark_for_class():
items = self.items_from_class_label(marker_name, item)
for item_for_label in items:
rel_mark = RelativeMark(
item_for_label, item, move_after=is_after
)
if is_after:
self.rel_marks.append(rel_mark)
else:
self.rel_marks.insert(0, rel_mark)
item.inc_rel_marks()
if not items:
items = self.items_from_path_label(marker_name, item)
for item_for_label in items:
rel_mark = RelativeMark(
item_for_label, item, move_after=is_after
Expand All @@ -258,8 +283,8 @@ def is_mark_for_class() -> bool:
self.rel_marks.append(rel_mark)
else:
self.rel_marks.insert(0, rel_mark)
item.inc_rel_marks()
return len(items) > 0
item.inc_rel_marks()
return len(items) > 0
return False

def handle_relative_marks(self, item: Item, mark: Mark) -> bool:
Expand Down