update messages in status - #99
ianthomas23 merged 4 commits into
Conversation
Sorry, something went wrong.
|
|
||
| // TODO: check if this message should be displayed even if there are untracked files | ||
| if (!(sl.has_tobecommited_header() | sl.has_notstagged_header() | sl.has_unmerged_header() | sl.has_untracked_header())) | ||
| if (is_long & (!(sl.has_tobecommited_header() | sl.has_notstagged_header() | sl.has_unmerged_header() | sl.has_untracked_header()))) |
There was a problem hiding this comment.
| if (is_long & (!(sl.has_tobecommited_header() | sl.has_notstagged_header() | sl.has_unmerged_header() | sl.has_untracked_header()))) | |
| if (is_long & !(sl.has_tobecommited_header() | sl.has_notstagged_header() | sl.has_unmerged_header() | sl.has_untracked_header())) |
Sorry, something went wrong.
| @pytest.mark.parametrize("short_flag", ["", "-s"]) | ||
| def test_status_typechange(xtl_clone, git2cpp_path, tmp_path, short_flag): | ||
| """Test status shows typechange (file to symlink or vice versa)""" | ||
| # Note: This test may need to be skipped on Windows |
There was a problem hiding this comment.
I'd rather not mention Windows at all. We say in the top-level README that git2cpp "can be compiled and used on any POSIX-compliant system" so nobody on Windows should get this far.
Sorry, something went wrong.
| os.remove(del_file) | ||
|
|
||
| # Stage the two previous files | ||
| subprocess.run([git2cpp_path, "add", "staged.txt", "README.md"], cwd=xtl_path) |
There was a problem hiding this comment.
I'd prefer to always either use check=True or check the returncode is 0 on all subprocess.run calls, otherwise we'll see an error reported later in such tests that won't be as easily understandable as the root cause of the problem.
Sorry, something went wrong.
There was a problem hiding this comment.
We'll go for returncode then, because somethime we check if it's non zero.
Sorry, something went wrong.
| repo_path.mkdir() | ||
|
|
||
| # Initialize repo | ||
| subprocess.run([git2cpp_path, "init"], cwd=repo_path) |
There was a problem hiding this comment.
Check returncode or use check=True.
Sorry, something went wrong.
|
|
||
| # Clone it | ||
| clone_path = tmp_path / "clone" | ||
| subprocess.run(["git", "clone", str(repo_path), str(clone_path)]) |
There was a problem hiding this comment.
And here.
Sorry, something went wrong.
| # Changes to be committed | ||
| staged = xtl_path / "staged.txt" | ||
| staged.write_text("staged") | ||
| subprocess.run([git2cpp_path, "add", "staged.txt"], cwd=xtl_path) |
There was a problem hiding this comment.
And here.
Sorry, something went wrong.
|
|
||
| // TODO: check if this message should be displayed even if there are untracked files | ||
| if (!(sl.has_tobecommited_header() | sl.has_notstagged_header() | sl.has_unmerged_header() | sl.has_untracked_header())) | ||
| if (is_long & (!(sl.has_tobecommited_header() | sl.has_notstagged_header() | sl.has_unmerged_header() | sl.has_untracked_header()))) |
There was a problem hiding this comment.
We should use boolean operators (&&, ||) instead of bitwise operators here (&, |) toi avoid unecessary conversion to integers.
Sorry, something went wrong.
There was a problem hiding this comment.
I published the review after the suggestion was applied, thus the outdated tag, but the comment is still relevant.
Sorry, something went wrong.
| if (is_long & !sl.has_tobecommited_header() & (sl.has_notstagged_header() | sl.has_untracked_header())) | ||
| { | ||
| std::cout << nothingtocommit_message << std::endl; | ||
| } |
There was a problem hiding this comment.
The function is quite long, it would make it easier to read if it was split into logical smaller functions.
Sorry, something went wrong.
ianthomas23
left a comment
There was a problem hiding this comment.
One more change, then looks good to go.
Sorry, something went wrong.
ianthomas23
left a comment
There was a problem hiding this comment.
Thanks @SandrineP
Sorry, something went wrong.
Update messages in status using new implemented commands
Add tracking info
Add more tests