rorybyrne / git-plan

Git Plan - a better workflow for git
MIT License
182 stars 5 forks source link

Consider using git config editor rather than a separate environment variable named EDITOR #84

Closed akashRindhe closed 3 years ago

akashRindhe commented 3 years ago

Current Situation

git-plan looks for the user's choice of editor through an environment variable named EDITOR and if such a variable doesn't exist, it defaults to vim.

Enhancement

git users have their choice of editor pre-configured which can be found by running git config --global core.editor. Consider using this configuration.

Reasoning

Reduces the effort for users in having to setup a new environment variable when one already exists in the git domain.

Implementation

Look up the result of git config --global core.editor and default to vim if variable is not set.

Goorzhel commented 3 years ago

EDITOR is a pretty ubiquitous env var, but here's a first pass (since there's only one place an editor is looked for):

diff --git a/git_plan/service/plan.py b/git_plan/service/plan.py
index cb60191..8ba2bfb 100644
--- a/git_plan/service/plan.py
+++ b/git_plan/service/plan.py
@@ -4,9 +4,9 @@
 """
 import os
 import tempfile
 import time
-from subprocess import call
+from subprocess import call, getoutput
 from typing import List

 from git_plan.exceptions import PlanEmpty
 from git_plan.model.commit import Commit, CommitMessage
@@ -87,9 +87,11 @@ class PlanService:
     def _prompt_user_for_plan(self, initial: str = None) -> CommitMessage:
         if not initial:
             initial = self._plan_template

-        editor = os.environ.get('EDITOR', 'vim')
+        editor = getoutput('git config --global core.editor')
+        if not editor:
+            editor = os.environ.get('EDITOR', 'vim')
         if not is_installed(editor):
             raise RuntimeError("Couldn't find an editor installed on your system.")

         with tempfile.NamedTemporaryFile(suffix=".tmp", mode='r+') as file:
rorybyrne commented 3 years ago

This sounds reasonable, it makes sense to use existing git configuration wherever possible.

I'm happy to take a PR for this but the subprocess call should be a method in the GitService.

akashRindhe commented 3 years ago

Fixed in https://github.com/synek/git-plan/pull/85, so closing this issue.