feat(commands/commit): apply prepare-commit-msg hook #250

saygox · 2020-08-14T08:54:07Z

Types of changes

Description

invoke cz commit from git prepare-commit-msg hook

Checklist

Add test cases to all the changes you introduce
Run ./script/format and ./script/test locally to ensure this change passes linter check and test
Test the changes on the local machine manually
Update the documentation for the changes

Steps to Test This Pull Request

Create a fake repo
Add prepare-commit-msg hook
Start comitting

Expected behavior

hook invokes cz commit
Editor is invoked after questionary
commit message is filled by cz commit

Additional context

cz commit with prepare-commit-msg hook #249
test only in linux

codecov · 2020-08-14T09:43:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (65645e0) 97.92% compared to head (1053d01) 97.97%.
Report is 617 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #250      +/-   ##
==========================================
+ Coverage   97.92%   97.97%   +0.05%     
==========================================
  Files          39       43       +4     
  Lines        1395     1432      +37     
==========================================
+ Hits         1366     1403      +37     
  Misses         29       29

Flag	Coverage Δ
unittests	`97.97% <100.00%> (+0.05%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Lee-W · 2020-08-20T10:37:41Z

Hi, although I've not yet had the time to fully review the code, I just test it on my local machine and failed.

This is the .pre-commit-config.yaml I used.

default_stages: [push]
repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v3.1.0
    hooks:
      - id: check-vcs-permalinks
      - id: end-of-file-fixer
      - id: trailing-whitespace
        args: [--markdown-linebreak-ext=md]
      - id: debug-statements
      - id: no-commit-to-branch

  - repo: https://github.com/saygox/commitizen
    rev: 09fa312b7846850f44f7860632685189e37c1bec
    hooks:
      - id: commitizen-prepare-commit-msg
        stages: [prepare-commit-msg

The command I used.

mkdir test
cd test
git init

# add .pre-commit-config.yaml

pre-commit install -t prepare-commit-msg
git commt

The error message

  File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/bin/cz", line 8, in <module>
    sys.exit(main())
  File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/commitizen/cli.py", line 286, in main
    args.func(conf, vars(args))()
  File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/commitizen/commands/commit.py", line 94, in __call__
    m = self.prompt_commit_questions()
  File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/commitizen/commands/commit.py", line 63, in prompt_commit_questions
    answers = questionary.prompt(questions, style=cz.style)
  File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/questionary/prompt.py", line 97, in prompt
    answer = question.unsafe_ask(patch_stdout)
  File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/questionary/question.py", line 59, in unsafe_ask
    return self.application.run()
  File "/.../.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.7/site-packages/prompt_toolkit/application/application.py", line 816, in run
    self.run_async(pre_run=pre_run, set_exception_handler=set_exception_handler)

Python version: 3.7
OS: macOS 10.15

ShaneKosieradzki · 2021-01-05T23:55:13Z

Glad to see this is being implemented as it is the biggest thing preventing us from adopting this technology. Is there a timeline for this PR?

Also @Lee-W not sure if this error occurred when you were creating the comment on this PR but you have a typo in the stages section of your .pre-commit-config.yaml for the commitizen-prepare-commit-msg hook. You are missing a closing ]. Perhaps that is what caused your error when running on your local machine.

Lee-W · 2021-01-07T04:12:42Z

@ShaneKosieradzki Thanks for reminding me! But I still encounter an error on my local machine. Did you succeed on your local machine?

Traceback (most recent call last):
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/bin/cz", line 8, in <module>
    sys.exit(main())
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/commitizen/cli.py", line 286, in main
    args.func(conf, vars(args))()
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/commitizen/commands/commit.py", line 95, in __call__
    m = self.prompt_commit_questions()
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/commitizen/commands/commit.py", line 63, in prompt_commit_questions
    answers = questionary.prompt(questions, style=cz.style)
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/questionary/prompt.py", line 97, in prompt
    answer = question.unsafe_ask(patch_stdout)
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/questionary/question.py", line 59, in unsafe_ask
    return self.application.run()
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 814, in run
    return loop.run_until_complete(
  File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 781, in run_async
    return await _run_async2()
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 763, in _run_async2
    result = await _run_async()
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/application/application.py", line 694, in _run_async
    with self.input.raw_mode(), self.input.attach(
  File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "/Users/weilee/.cache/pre-commit/repof8ns4eua/py_env-python3/lib/python3.8/site-packages/prompt_toolkit/input/vt100.py", line 161, in _attached_input
    loop.add_reader(fd, callback)
  File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/asyncio/selector_events.py", line 332, in add_reader
    return self._add_reader(fd, callback, *args)
  File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/asyncio/selector_events.py", line 261, in _add_reader
    self._selector.register(fd, selectors.EVENT_READ,
  File "/Users/weilee/.pyenv/versions/3.8.5/lib/python3.8/selectors.py", line 522, in register
    self._selector.control([kev], 0, 0)
OSError: [Errno 22] Invalid argument

lobotmcj · 2021-08-01T19:08:43Z

@Lee-W it worked for me (Ubuntu 18.04.5; python 3.6.9; pre-commit 2.13.0) using pretty much your steps (with the closing ']' fix).
Exact steps:

make test
cd test
python3 -m venv venv
source venv/bin/activate
pip install pre-commit
vim .pre-commit-config.yaml

repos:                                                                                                                                                                                                             
  - repo: https://github.com/saygox/commitizen                                                                                                                                                                     
    rev: 09fa312b7846850f44f7860632685189e37c1bec                                                                                                                                                                  
    hooks:                                                                                                                                                                                                         
      - id: commitizen-prepare-commit-msg                                                                                                                                                                          
        stages: [prepare-commit-msg]

git init
git add .pre-commit-config.yaml
pre-commit install --install-hooks -t prepare-commit-msg
git commit

I did encounter one potential issue in playing around a bit, however, after successfully commiting.

git commit --amend

commitizen prepare commit msg............................................No files added to staging!
Failed
- hook id: commitizen-prepare-commit-msg
- exit code: 11

I'd like to see this issue/PR continued -- note that it would also "solve", or at least provide a viable alternative to, #248 since you are actually using git commit and therefore any flags or configs that you want for that, such as -S or --signoff.

Lee-W · 2021-08-07T15:38:31Z

Hi @lobotmcj , I still encounter the same error on my mac. Still have no idea on how this could be fixed. In addition, we might need to take care of windows cases as well

Lee-W · 2021-08-08T02:58:30Z

@woile Do you have any though on this one?

Lee-W · 2021-11-07T04:22:09Z

Hi @saygox thanks for your update. Could you please help us rebase the latest change from the master branch? I've tried to do so but fail to pass the test. Also, I'm wondering is it excepted to call git.commit twice on line 143 in commitizen/commands/commit.py

saygox · 2021-11-07T05:40:26Z

Hi @Lee-W san.
I tried to rebase this branch from HEAD. but my operation is not good so I can't push it.
So I push the rebased branch to https://github.com/saygox/commitizen/tree/feature/prepare-commit-msg2 .

sorry to the line 143's git.commit is miss. prepare-commit-msg2 branch is fixed it.
Would you mind to tell me how to merge a new rebased one to this request ? (or need to create a new push request?)

Lee-W · 2021-11-07T06:12:51Z

Hi @saygox , have you try git push origin feature/prepare-commit-msg -f?

Lee-W · 2021-11-07T14:25:28Z

commitizen/commands/commit.py

        out.info(f"\n{m}\n")

        if dry_run:
            raise DryRunExit()

+        if commit_msg_file:
+            defaultmesaage = ""


nitpick: I'd suggest using default_message

docs/README.md

Lee-W · 2021-11-23T00:59:12Z

@saygox Thanks for your hard work. I'm a bit busy lately, but I'll start digging into it these days.

Lee-W

Thanks for your hard work! I just tried it on my local machine and git commit with the pre-commit hook works well 🎉 but I just found out the if you run cz commit it will trigger the questionary twice as under the hook cz commit calls git commit as well. I think we might need to address this issue.

Also, I'm thinking of making wrap_studio a package.

wrap_studio/
    __init__.py
    unix.py
    windows.py
    linux.py

What do you think?

Lee-W · 2021-11-24T14:39:28Z

commitizen/wrap_stdio_linux.py

+if sys.platform == "linux":  # pragma: no cover
+    import os
+
+    # from io import IOBase


As this is not used, please remove this line

line 6 is not need. I remove it.
line 3, 4 are needs for git hub release test. this details will be explained in the next item.

Lee-W · 2021-11-24T14:40:33Z

commitizen/wrap_stdio_linux.py

@@ -0,0 +1,61 @@
+import sys
+
+if sys.platform == "linux":  # pragma: no cover


Why do we need to check this in both commitizen/wrap_stdio.py and here?

The coverage of pytest is check all files.
if it is removed from wrap_stdio_linux.py, the git hub release test for mac os is not passed.

---------- coverage: platform darwin, python 3.7.3-final-0 ----------- Name Stmts Miss Cover Missing ------------------------------------------------------------------------------------------ : commitizen/wrap_stdio_linux.py 6 2 67% 12, 16

and same reason.
wrap_stdio_unix.py is checked on the linux environment.

---------- coverage: platform linux, python 3.8.10-final-0 ----------- Name Stmts Miss Cover Missing ------------------------------------------------------------------------------------------ : commitizen/wrap_stdio_unix.py 42 42 0% 1-73

Got it. From my point of view, we should write cleaner code instead of just trying to match the check. I'm ok with skipping part of the coverage check if it does make the code cleaner.

Lee-W · 2021-11-24T14:42:19Z

commitizen/wrap_stdio_unix.py

+        def __del__(self):
+            self.tty.close()
+
+    # backup_event_loop = None


As this is not used, please remove this line

I will remove it.

Lee-W · 2021-11-24T14:46:40Z

tests/test_wrap_stdio.py

+
+        assert sys.stdout == tmp_stdout
+        assert sys.stderr == tmp_stderr
+


please remove the redundant blank line

I will remove it.

saygox · 2021-12-07T06:10:52Z

Thank you for your good comment and sorry to my late reply.

next commit is bellow.

Wrap standard in/out files turn to the package.
cz commit will be walk well.

Lee-W · 2021-12-07T14:29:43Z

It seems the pipeline is due to lack of runner which is our side of the problem. I'll take a look at it these days and see how we can tackle it.

Lee-W · 2021-12-08T02:48:25Z

It seems the pipeline is due to lack of runner which is our side of the problem. I'll take a look at it these days and see how we can tackle it.

This PR #456 should address this issue

add --commit-msg-file argument

for pass the python-check (3.8)

This reverts commit f7f80bb.

Lee-W · 2021-12-22T15:15:28Z

I just tried the latest version on my Mac but failed

Failed
- hook id: commitizen-prepare-commit-msg
- exit code: 1

also, we'll need to address #250 (review)

saygox · 2021-12-23T15:07:00Z

Hi, @Lee-W san.
Sorry to this trouble.

I tell you about sad thing.
I am far from Mac, so I don't know about Mac OS specification well.
For this PR I bought used Mac mini(2014) and OS is 10.15.7.(new Mac is expensive)
This programming woks well my Mac.

I try to upgrade my Mac to latest OS, but hung and reboot.
So in this time I have no way to what is happen.

Would you mind tell me more information about error.

Lee-W · 2021-12-24T03:36:05Z

@saygox Sure! I was trying to dig a bit but didn't find out the cause. Will run a deeper investigation when I have time. Thanks!

Lee-W mentioned this pull request Jan 4, 2021

Auto-run Commitizen on git commit #330

Closed

Lee-W mentioned this pull request Sep 25, 2021

cz commit with prepare-commit-msg hook #249

Open

Lee-W mentioned this pull request Oct 20, 2021

Support for git --hooks #450

Closed

Lee-W added the os: macOS has issue on macOS label Oct 28, 2021

saygox force-pushed the feature/prepare-commit-msg branch from 1a3ffb6 to cc7066c Compare November 7, 2021 06:14

Lee-W reviewed Nov 7, 2021

View reviewed changes

saygox force-pushed the feature/prepare-commit-msg branch from 1a947d1 to 04b860d Compare November 10, 2021 15:12

saygox force-pushed the feature/prepare-commit-msg branch from f4ac267 to 3a15639 Compare November 23, 2021 00:32

Lee-W removed the os: macOS has issue on macOS label Nov 24, 2021

Lee-W reviewed Nov 24, 2021

View reviewed changes

saygox added 6 commits December 8, 2021 21:19

feat(commands/commit): apply prepare-commit-msg hook

788e368

add --commit-msg-file argument

test(commands/commit): set staging_is_clean to pre_commit_msg_hook

c658602

for pass the python-check (3.8)

feat(commands/commit): apply prepare-commit-msg hook for Mac

457b42e

test(commands/commit): change prepare-commit-msg hook for Mac

6ea8720

refactor(commands/commit): change valiable by comment and use black

1abcc02

refactor(iest/commands/commit): use black

888ba83

saygox added 12 commits December 8, 2021 21:19

refactor(commands/commit): use isort

73298f2

refactor(commands/commit) use direct async class and open tty as binmod

db49ffb

refactor(commands/commit) ignore type worrn

d6a73af

feat(wrap_stdio): sepalate wrap stdio module

9fe0fad

feat(commands/commit): apply prepare-commit-msg hook for Win

828a2d6

test(wrap_stdio): does not load other platform

2a33ba0

test(test_wrap_stdio): add wrap_stdio tests

13c6781

refactor(wrap_stdio): remake classes

967131f

fix(commands/commit): check cz commit msg

e36d432

refactor: wrap_stdio

35050e7

Revert "refactor: wrap_stdio"

a70f23b

This reverts commit f7f80bb.

refactor(wrap_stdout): remove unused lines

14f4182

saygox force-pushed the feature/prepare-commit-msg branch from 6112f8c to 14f4182 Compare December 8, 2021 12:35

saygox added 2 commits December 11, 2021 14:50

style(test/wrap_stdout): format by black

97876a5

test(commands/commit): add a test for is_blank_commit_file

1053d01

Lee-W added pr-status: reviewing pr-status: wait-for-review and removed pr-status: reviewing labels Mar 13, 2022

crai0 mentioned this pull request Apr 28, 2023

feat(commit): add --write-message-to-file option #731

Merged

4 tasks

Lee-W added the pr-status: wait-for-modification label Dec 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(commands/commit): apply prepare-commit-msg hook #250

feat(commands/commit): apply prepare-commit-msg hook #250

saygox commented Aug 14, 2020

codecov bot commented Aug 14, 2020 •

edited

Lee-W commented Aug 20, 2020 •

edited

ShaneKosieradzki commented Jan 5, 2021

Lee-W commented Jan 7, 2021

lobotmcj commented Aug 1, 2021 •

edited

Lee-W commented Aug 7, 2021

Lee-W commented Aug 8, 2021

Lee-W commented Nov 7, 2021

saygox commented Nov 7, 2021

Lee-W commented Nov 7, 2021 •

edited

Lee-W Nov 7, 2021

Lee-W commented Nov 23, 2021

Lee-W left a comment

Lee-W Nov 24, 2021

saygox Dec 7, 2021

Lee-W Nov 24, 2021

saygox Dec 7, 2021

saygox Dec 7, 2021

Lee-W Dec 7, 2021

Lee-W Nov 24, 2021

saygox Dec 7, 2021

Lee-W Nov 24, 2021

saygox Dec 7, 2021

saygox commented Dec 7, 2021

Lee-W commented Dec 7, 2021

Lee-W commented Dec 8, 2021

Lee-W commented Dec 22, 2021

saygox commented Dec 23, 2021

Lee-W commented Dec 24, 2021

		@@ -0,0 +1,61 @@
		import sys

		if sys.platform == "linux": # pragma: no cover


		assert sys.stdout == tmp_stdout
		assert sys.stderr == tmp_stderr

feat(commands/commit): apply prepare-commit-msg hook #250

Are you sure you want to change the base?

feat(commands/commit): apply prepare-commit-msg hook #250

Conversation

saygox commented Aug 14, 2020

Types of changes

Description

Checklist

Steps to Test This Pull Request

Expected behavior

Additional context

codecov bot commented Aug 14, 2020 • edited

Codecov Report

Lee-W commented Aug 20, 2020 • edited

ShaneKosieradzki commented Jan 5, 2021

Lee-W commented Jan 7, 2021

lobotmcj commented Aug 1, 2021 • edited

Lee-W commented Aug 7, 2021

Lee-W commented Aug 8, 2021

Lee-W commented Nov 7, 2021

saygox commented Nov 7, 2021

Lee-W commented Nov 7, 2021 • edited

Choose a reason for hiding this comment

Lee-W commented Nov 23, 2021

Lee-W left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saygox commented Dec 7, 2021

Lee-W commented Dec 7, 2021

Lee-W commented Dec 8, 2021

Lee-W commented Dec 22, 2021

saygox commented Dec 23, 2021

Lee-W commented Dec 24, 2021

codecov bot commented Aug 14, 2020 •

edited

Lee-W commented Aug 20, 2020 •

edited

lobotmcj commented Aug 1, 2021 •

edited

Lee-W commented Nov 7, 2021 •

edited