Repository: VirtusLab/git-machete
Pull request #1379 (2024-12-17 12:22:39)

Tidy up implementation for annotations

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/VirtusLab/git-machete/pull/1379?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VirtusLab) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 98.47%. Comparing base [(`b5be9e1`)](https://app.codecov.io/gh/VirtusLab/git-machete/commit/b5be9e1e71bda6a799c035814c2fdb6ed2e85378?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VirtusLab) to head [(`62bfdac`)](https://app.codecov.io/gh/VirtusLab/git-machete/commit/62bfdac51cc4c009ad33124e88bf1570b549fff4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VirtusLab).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff            @@
##           develop    #1379   +/-   ##
========================================
  Coverage    98.47%   98.47%           
========================================
  Files           14       14           
  Lines         4007     4007           
  Branches       782      783    +1     
========================================
  Hits          3946     3946           
  Misses          39       39           
  Partials        22       22           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/VirtusLab/git-machete/pull/1379?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VirtusLab).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VirtusLab).


Patch:
diff --git a/docs/man/git-machete.1 b/docs/man/git-machete.1
index 5a6197e..c79fb82 100644
--- a/docs/man/git-machete.1
+++ b/docs/man/git-machete.1
@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
 ..
-.TH "GIT-MACHETE" "1" "Dec 13, 2024" "" "git-machete"
+.TH "GIT-MACHETE" "1" "Dec 17, 2024" "" "git-machete"
 .SH NAME
 git-machete \- git-machete 3.31.2
 .sp
diff --git a/git_machete/annotation.py b/git_machete/annotation.py
index 07841fd..2ee14e3 100644
--- a/git_machete/annotation.py
+++ b/git_machete/annotation.py
@@ -1,87 +1,73 @@
 import re
+from typing import Callable, NamedTuple
 
 from git_machete import utils
 
 
-class Qualifiers:
-    def __init__(self, annotation: str):
-        self.__annotation_without_qualifiers: str = annotation
-        self.__push_text: str = ''
-        self.__rebase_text: str = ''
-        self.__slide_out_text: str = ''
-        self.__update_with_merge_text: str = ""
-        self.rebase: bool = True
-        self.push: bool = True
-        self.slide_out: bool = True
-        self.update_with_merge: bool = False
+class Qualifiers(NamedTuple):
+    rebase: bool = True
+    push: bool = True
+    slide_out: bool = True
+    update_with_merge: bool = False
 
-        def match_pattern(pattern: str) -> str:
-            return f".*\\b{pattern}\\b.*"
+    def is_default(self) -> bool:
+        return self.rebase and self.push and self.slide_out and not self.update_with_merge
 
-        def sub_pattern(pattern: str) -> str:
-            return f"[ ]?{pattern}[ ]?"
+    def is_non_default(self) -> bool:
+        return not self.is_default()
 
-        rebase_anno = 'rebase=no'
-        rebase_match = re.match(match_pattern(rebase_anno), annotation)
-        if rebase_match:
-            self.rebase = False
-            self.__rebase_text = rebase_anno
-            self.__annotation_without_qualifiers = re.sub(sub_pattern(rebase_anno), ' ', self.__annotation_without_qualifiers)
+    def __str__(self) -> str:
+        segments = ["rebase=no" if not self.rebase else None,
+                    "push=no" if not self.push else None,
+                    "slide-out=no" if not self.slide_out else None,
+                    "update=merge" if self.update_with_merge else None]
+        return ' '.join(filter(None, segments))
 
-        push_anno = 'push=no'
-        push_match = re.match(match_pattern(push_anno), annotation)
-        if push_match:
-            self.push = False
-            self.__push_text = push_anno
-            self.__annotation_without_qualifiers = re.sub(sub_pattern(push_anno), ' ', self.__annotation_without_qualifiers)
 
-        slide_out_anno = 'slide-out=no'
-        slide_out_match = re.match(match_pattern(slide_out_anno), annotation)
-        if slide_out_match:
-            self.slide_out = False
-            self.__slide_out_text = slide_out_anno
-            self.__annotation_without_qualifiers = re.sub(sub_pattern(slide_out_anno), ' ', self.__annotation_without_qualifiers)
+class Annotation(NamedTuple):
+    text_without_qualifiers: str
+    qualifiers: Qualifiers
 
-        update_with_merge_anno = 'update=merge'
-        update_with_merge_match = re.match(match_pattern(update_with_merge_anno), annotation)
-        if update_with_merge_match:
-            self.update_with_merge = True
-            self.__update_with_merge_text = update_with_merge_anno
-            self.__annotation_without_qualifiers = re.sub(sub_pattern(update_with_merge_anno), ' ', self.__annotation_without_qualifiers)
-
-    def get_annotation_text_without_qualifiers(self) -> str:
-        return self.__annotation_without_qualifiers.strip()
-
-    def get_qualifiers_text(self) -> str:
-        text = f'{self.__rebase_text} {self.__push_text} {self.__slide_out_text} {self.__update_with_merge_text}'
-        return re.sub(' +', ' ', text).strip()
-
-
-class Annotation:
-    def __init__(self, text: str):
-        self.text = text.strip()
-        self.qualifiers = Qualifiers(text)
-        self.text_without_qualifiers = self.qualifiers.get_annotation_text_without_qualifiers()
-        self.qualifiers_text = self.qualifiers.get_qualifiers_text()
-
-    def get_unformatted_text(self) -> str:
-        if not (self.text_without_qualifiers or self.qualifiers_text):
+    @property
+    def unformatted_full_text(self) -> str:
+        if not (self.text_without_qualifiers or self.qualifiers.is_non_default()):
             return ''
-        annotation_text = ' '
+        result = ''
         if self.text_without_qualifiers:
-            annotation_text += self.text_without_qualifiers
-        if self.qualifiers_text:
-            annotation_text += ' ' if self.text_without_qualifiers else ''
-            annotation_text += self.qualifiers_text
-        return annotation_text
+            result += self.text_without_qualifiers
+        if self.text_without_qualifiers and self.qualifiers.is_non_default():
+            result += ' '
+        if self.qualifiers.is_non_default():
+            result += str(self.qualifiers)
+        return result
 
-    def get_formatted_text(self) -> str:
-        if not (self.text_without_qualifiers or self.qualifiers_text):
+    @property
+    def formatted_full_text(self) -> str:
+        if not (self.text_without_qualifiers or self.qualifiers.is_non_default()):
             return ''
-        annotation_text = "  "
+        result = ''
         if self.text_without_qualifiers:
-            annotation_text += utils.dim(self.text_without_qualifiers)
-        if self.qualifiers_text:
-            annotation_text += ' ' if self.text_without_qualifiers else ''
-            annotation_text += utils.dim(utils.underline(s=self.qualifiers_text))
-        return annotation_text
+            result += utils.dim(self.text_without_qualifiers)
+        if self.text_without_qualifiers and self.qualifiers.is_non_default():
+            result += ' '
+        if self.qualifiers.is_non_default():
+            result += utils.dim(utils.underline(s=str(self.qualifiers)))
+        return result
+
+    @staticmethod
+    def parse(text_with_qualifiers: str) -> "Annotation":
+        text_without_qualifiers = text_with_qualifiers
+        qualifiers = Qualifiers()
+
+        def parse_one(pattern: str, qf: Callable[[Qualifiers], Qualifiers]) -> None:
+            nonlocal qualifiers, text_without_qualifiers
+            match = re.match(f".*\\b{pattern}\\b.*", text_without_qualifiers)
+            if match:
+                qualifiers = qf(qualifiers)
+                text_without_qualifiers = re.sub(f"[ ]?{pattern}[ ]?", ' ', text_without_qualifiers)
+
+        parse_one('rebase=no', lambda q: q._replace(rebase=False))
+        parse_one('push=no', lambda q: q._replace(push=False))
+        parse_one('slide-out=no', lambda q: q._replace(slide_out=False))
+        parse_one('update=merge', lambda q: q._replace(update_with_merge=True))
+        return Annotation(text_without_qualifiers.strip(), qualifiers)
diff --git a/git_machete/client.py b/git_machete/client.py
index b971c95..e5375ff 100644
--- a/git_machete/client.py
+++ b/git_machete/client.py
@@ -12,7 +12,7 @@ from typing import (Callable, Dict, Iterator, List, Optional, Set, Tuple, Type,
                     TypeVar)
 
 from . import git_config_keys, utils
-from .annotation import Annotation
+from .annotation import Annotation, Qualifiers
 from .code_hosting import (CodeHostingClient, CodeHostingSpec,
                            OrganizationAndRepository,
                            OrganizationAndRepositoryAndRemote, PullRequest,
@@ -199,7 +199,7 @@ class MacheteClient:
                                                                        line.strip().split(" ", 1)]
             branch = branch_and_maybe_annotation[0]
             if len(branch_and_maybe_annotation) > 1:
-                self.__annotations[branch] = Annotation(branch_and_maybe_annotation[1])
+                self.__annotations[branch] = Annotation.parse(branch_and_maybe_annotation[1])
             if branch in self.managed_branches:
                 raise MacheteException(
                     f"{self.__branch_layout_file_path}, line {index + 1}: branch "
@@ -298,7 +298,7 @@ class MacheteClient:
             self.__indent = "  "
 
         def render_dfs(branch: LocalBranchShortName, depth: int) -> List[str]:
-            annotation = self.__annotations[branch].get_unformatted_text() if branch in self.__annotations else ""
+            annotation = (" " + self.__annotations[branch].unformatted_full_text) if branch in self.__annotations else ""
             assert self.__indent is not None
             res: List[str] = [depth * self.__indent + branch + annotation]
             for down_branch in self.__down_branches.get(branch) or []:
@@ -424,12 +424,12 @@ class MacheteClient:
         if branch in self.__annotations and words == ['']:
             del self.__annotations[branch]
         else:
-            self.__annotations[branch] = Annotation(" ".join(words))
+            self.__annotations[branch] = Annotation.parse(" ".join(words))
         self.save_branch_layout_file()
 
     def print_annotation(self, branch: LocalBranchShortName) -> None:
         if branch in self.__annotations:
-            print(self.__annotations[branch].text)
+            print(self.__annotations[branch].text_without_qualifiers)
 
     def update(
             self, *, opt_merge: bool, opt_no_edit_merge: bool,
@@ -486,7 +486,7 @@ class MacheteClient:
         self.__up_branch = {}
         self.__indent = "  "
         for branch in self.annotations.keys():
-            self.annotations[branch].text_without_qualifiers = ''
+            self.annotations[branch] = self.annotations[branch]._replace(text_without_qualifiers='')
 
         root_of = dict((branch, branch) for branch in all_local_branches)
 
@@ -1201,8 +1201,8 @@ class MacheteClient:
                 current = bold(branch)
 
             anno: str = ''
-            if branch in self.__annotations:
-                anno = self.__annotations[branch].get_formatted_text()
+            if branch in self.__annotations and self.__annotations[branch].formatted_full_text:
+                anno = '  ' + self.__annotations[branch].formatted_full_text
 
             s, remote = self.__git.get_combined_remote_sync_status(branch)
             sync_status = {
@@ -1761,20 +1761,21 @@ class MacheteClient:
                          f'than in machete file ({bold(upstream) if upstream else "<none, is a root>"})')
                     anno += (f" WRONG {spec.pr_short_name} {spec.base_branch_name.upper()} or MACHETE PARENT? "
                              f"{spec.pr_short_name} has {pr.base}")
-                old_annotation_text, old_annotation_qualifiers_text = '', ''
+                old_annotation_text = ''
+                old_annotation_qualifiers = Qualifiers()
                 if LocalBranchShortName.of(pr.head) in self.__annotations:
-                    old_annotation_text = self.__annotations[LocalBranchShortName.of(pr.head)].text_without_qualifiers
-                    old_annotation_qualifiers_text = self.__annotations[LocalBranchShortName.of(pr.head)].qualifiers_text
+                    old_annotation = self.__annotations[LocalBranchShortName.of(pr.head)]
+                    old_annotation_text = old_annotation.text_without_qualifiers
+                    old_annotation_qualifiers = old_annotation.qualifiers
 
-                if pr.user != current_user and old_annotation_qualifiers_text == '':
+                if pr.user != current_user and old_annotation_qualifiers.is_default():
                     if verbose:
                         print(fmt(f'Annotating {bold(pr.head)} as `{anno} rebase=no push=no`'))
-                    self.__annotations[LocalBranchShortName.of(pr.head)] = Annotation(f'{anno} rebase=no push=no')
+                    self.__annotations[LocalBranchShortName.of(pr.head)] = Annotation(anno, Qualifiers(rebase=False, push=False))
                 elif old_annotation_text != anno:
                     if verbose:
                         print(fmt(f'Annotating {bold(pr.head)} as `{anno}`'))
-                    self.__annotations[LocalBranchShortName.of(pr.head)] = Annotation(f'{anno} {old_annotation_qualifiers_text}') \
-                        if old_annotation_text is not None else Annotation(anno)
+                    self.__annotations[LocalBranchShortName.of(pr.head)] = Annotation(anno, old_annotation_qualifiers)
             else:
                 debug(f'{pr} does NOT correspond to a managed branch')
         self.save_branch_layout_file()
@@ -2535,11 +2536,9 @@ class MacheteClient:
             print(f'Description of {pr.display_text()} has been updated')
 
         current_user: Optional[str] = self.code_hosting_client.get_current_user_login()
-        if self.__annotations.get(head) and self.__annotations[head].qualifiers_text:
-            self.__annotations[head] = Annotation(self.__pull_request_annotation(spec, pr, current_user) + ' ' +
-                                                  self.__annotations[head].qualifiers_text)
-        else:
-            self.__annotations[head] = Annotation(self.__pull_request_annotation(spec, pr, current_user))
+        anno = self.__annotations.get(head)
+        self.__annotations[head] = Annotation(self.__pull_request_annotation(
+            spec, pr, current_user), anno.qualifiers if anno else Qualifiers())
         self.save_branch_layout_file()
 
         if opt_update_related_descriptions:
@@ -2900,7 +2899,7 @@ class MacheteClient:
             self.code_hosting_client.add_reviewers_to_pull_request(pr.number, reviewers)
             print(fmt(ok_str))
 
-        self.__annotations[head] = Annotation(self.__pull_request_annotation(spec, pr, current_user))
+        self.__annotations[head] = Annotation(self.__pull_request_annotation(spec, pr, current_user), qualifiers=Qualifiers())
         self.save_branch_layout_file()
 
         if opt_update_related_descriptions:


Test patch:
diff --git a/tests/test_client.py b/tests/test_client.py
index 5dc8754..123cb61 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -62,50 +62,50 @@ class TestClient(BaseTest):
         annotations: Dict[LocalBranchShortName, Annotation] = machete_client.annotations
 
         feature_2_branch = LocalBranchShortName.of('feature2')
-        assert annotations[feature_2_branch].text == 'annotation'
+        assert annotations[feature_2_branch].unformatted_full_text == 'annotation'
         assert annotations[feature_2_branch].qualifiers.rebase is True
         assert annotations[feature_2_branch].qualifiers.push is True
         assert annotations[feature_2_branch].text_without_qualifiers == 'annotation'
-        assert annotations[feature_2_branch].qualifiers_text == ''
+        assert str(annotations[feature_2_branch].qualifiers) == ''
 
         feature_3_branch = LocalBranchShortName.of('feature3')
-        assert annotations[feature_3_branch].text == 'annotation rebase=no push=no'
+        assert annotations[feature_3_branch].unformatted_full_text == 'annotation rebase=no push=no'
         assert annotations[feature_3_branch].qualifiers.rebase is False
         assert annotations[feature_3_branch].qualifiers.push is False
         assert annotations[feature_3_branch].text_without_qualifiers == 'annotation'
-        assert annotations[feature_3_branch].qualifiers_text == 'rebase=no push=no'
+        assert str(annotations[feature_3_branch].qualifiers) == 'rebase=no push=no'
 
         feature_4_branch = LocalBranchShortName.of('feature4')
-        assert annotations[feature_4_branch].text == 'rebase=no push=no annotation'
+        assert annotations[feature_4_branch].unformatted_full_text == 'annotation rebase=no push=no'
         assert annotations[feature_4_branch].qualifiers.rebase is False
         assert annotations[feature_4_branch].qualifiers.push is False
         assert annotations[feature_4_branch].text_without_qualifiers == 'annotation'
-        assert annotations[feature_4_branch].qualifiers_text == 'rebase=no push=no'
+        assert str(annotations[feature_4_branch].qualifiers) == 'rebase=no push=no'
 
         feature_5_branch = LocalBranchShortName.of('feature5')
-        assert annotations[feature_5_branch].text == 'annotation1 rebase=no annotation2 push=no annotation3'
+        assert annotations[feature_5_branch].unformatted_full_text == 'annotation1 annotation2 annotation3 rebase=no push=no'
         assert annotations[feature_5_branch].qualifiers.rebase is False
         assert annotations[feature_5_branch].qualifiers.push is False
         assert annotations[feature_5_branch].text_without_qualifiers == 'annotation1 annotation2 annotation3'
-        assert annotations[feature_5_branch].qualifiers_text == 'rebase=no push=no'
+        assert str(annotations[feature_5_branch].qualifiers) == 'rebase=no push=no'
 
         feature_6_branch = LocalBranchShortName.of('feature6')
-        assert annotations[feature_6_branch].text == 'annotation1 rebase=nopush=no annotation2'
+        assert annotations[feature_6_branch].unformatted_full_text == 'annotation1 rebase=nopush=no annotation2'
         assert annotations[feature_6_branch].qualifiers.rebase is True
         assert annotations[feature_6_branch].qualifiers.push is True
         assert annotations[feature_6_branch].text_without_qualifiers == 'annotation1 rebase=nopush=no annotation2'
-        assert annotations[feature_6_branch].qualifiers_text == ''
+        assert str(annotations[feature_6_branch].qualifiers) == ''
 
         feature_7_branch = LocalBranchShortName.of('feature7')
-        assert annotations[feature_7_branch].text == 'annotation1rebase=no push=noannotation2'
+        assert annotations[feature_7_branch].unformatted_full_text == 'annotation1rebase=no push=noannotation2'
         assert annotations[feature_7_branch].qualifiers.rebase is True
         assert annotations[feature_7_branch].qualifiers.push is True
         assert annotations[feature_7_branch].text_without_qualifiers == 'annotation1rebase=no push=noannotation2'
-        assert annotations[feature_7_branch].qualifiers_text == ''
+        assert str(annotations[feature_7_branch].qualifiers) == ''
 
         feature_8_branch = LocalBranchShortName.of('feature8')
-        assert annotations[feature_8_branch].text == 'annotation rebase=no push=no rebase=no push=no'
+        assert annotations[feature_8_branch].unformatted_full_text == 'annotation rebase=no push=no'
         assert annotations[feature_8_branch].qualifiers.rebase is False
         assert annotations[feature_8_branch].qualifiers.push is False
         assert annotations[feature_8_branch].text_without_qualifiers == 'annotation'
-        assert annotations[feature_8_branch].qualifiers_text == 'rebase=no push=no'
+        assert str(annotations[feature_8_branch].qualifiers) == 'rebase=no push=no'