Merge branch 'mayanksuman-master'

README.md

@@ -112,17 +112,17 @@ so:
 eg --examples-dir='the/default/dir' --custom-dir='my/fancy/dir' find
 ```
 
-Instead of doing this every time, you can define a configuration file. By
-default it is expected to live in `~/.egrc`. It must begin with a section
-called `eg-config` and can contain two keys: `custom-dir` and `examples-dir`.
-Here is an example of a valid config file:
+Instead of doing this every time, you can define a configuration file. It must
+begin with a section called `eg-config` and can contain two keys: `custom-dir`
+and `examples-dir`. Here is an example of a valid config file:
 
     [eg-config]
     examples-dir = ~/examples-dir
     custom-dir = ~/my/fancy/custom/dir
 
-Although by default the file is looked for at `~/.egrc`, you can also specify a
-different location at the command line like so:
+The config file is looked for first at `${XDG_CONFIG_HOME}/eg/egrc` and then at
+`~/.egrc`. You can also specify a different location at the command line like
+so:
 
 ```shell
 eg --config-file=myfile find

eg/config.py

@@ -154,19 +154,42 @@ def get_egrc_config(cli_egrc_path):
 
     If the egrc is not present, it returns an empty default Config.
 
-    This method tries to use the egrc at cli_egrc_path, then the default path.
+    This method tries to use the egrc at cli_egrc_path, then XDG_CONFIG_HOME,
+    and then the default path.
 
     cli_egrc_path: the path to the egrc as given on the command line via
         --config-file
     """
-    resolved_path = get_priority(cli_egrc_path, DEFAULT_EGRC_PATH, None)
-    expanded_path = get_expanded_path(resolved_path)
+    # We have to be a little crafty here, because we want to support people
+    # having an XDG_CONFIG_HOME dir that doesn't contain the config file. This
+    # will allow a smooth transition for XDG users that started using eg when we
+    # only supported `~/.egrc`. We don't want to suddenly break for them when we
+    # add new support.
+    config_path = ''
+
+    if cli_egrc_path:
+      # cli values always win
+      config_path = get_expanded_path(cli_egrc_path)
+
+    if config_path == '':
+      # Try for the xdg config.
+      xdg_home_dir = os.getenv('XDG_CONFIG_HOME')
+      if xdg_home_dir:
+        xdg_config_path = os.path.join(xdg_home_dir, 'eg', 'egrc')
+        xdg_config_path = get_expanded_path(xdg_config_path)
+        if os.path.isfile(xdg_config_path):
+          config_path = xdg_config_path
+
+    if config_path == '':
+      # Fall back to our home directory.
+      config_path = get_expanded_path(DEFAULT_EGRC_PATH)
+
 
     # Start as if nothing was defined in the egrc.
     egrc_config = get_empty_config()
 
-    if os.path.isfile(expanded_path):
-        egrc_config = get_config_tuple_from_egrc(expanded_path)
+    if os.path.isfile(config_path):
+        egrc_config = get_config_tuple_from_egrc(config_path)
 
     return egrc_config
 

test/config_test.py

@@ -168,27 +168,70 @@ def test_get_resolved_config_uses_custom_egrc_path(
 
 def test_get_egrc_config_reads_from_command_line():
     """
-    get_egrc_config should use the command line path if it is provided.
+    Favor the CLI config file.
     """
-    cli_path = 'path/from/command/line'
     expected = 'mock config from egrc'
 
     _assert_about_get_egrc_config(
-        cli_path=cli_path, path_to_expand=cli_path, expected_config=expected
+        cli_path='path/from/command/line',
+        cli_config_exists=True,
+        winning_config_path='path/from/command/line_expanded',
+        expected_config=expected,
+        home_config_exists=True,
+        xdg_dir_variable=os.path.join('path', 'to', 'xdg_home'),
+        xdg_config_exists=True,
     )
 
 
-def test_get_egrc_config_uses_default():
+def test_get_egrc_config_uses_home_dir_default_no_xdg_dir():
     """
-    get_egrc_config should use the default path if not provided on the command
-    line.
+    Fallback to the home directory config if there is no XDG directory.
     """
     expected = 'mock config from default position'
 
     _assert_about_get_egrc_config(
         cli_path=None,
-        path_to_expand=config.DEFAULT_EGRC_PATH,
+        cli_config_exists=False,
+        winning_config_path=os.path.join('~', '.egrc_expanded'),
+        home_config_exists=True,
         expected_config=expected,
+        xdg_dir_variable=None,
+        xdg_config_exists=False,
+    )
+
+
+def test_get_egrc_config_uses_home_dir_default_xdg_dir_file_not_present():
+    """
+    Fallback to the home directory config if XDG directory is set but the XDG
+    config file doesn't exist.
+    """
+    expected = 'mock config from default position'
+
+    _assert_about_get_egrc_config(
+        cli_path=None,
+        cli_config_exists=False,
+        winning_config_path=os.path.join('~', '.egrc_expanded'),
+        home_config_exists=True,
+        expected_config=expected,
+        xdg_dir_variable=os.path.join('path', 'to', 'xdg_home'),
+        xdg_config_exists=False,
+    )
+
+def test_get_egrc_config_uses_xdg_dir_default_if_present():
+    """
+    The XDG config should win over the home directory.
+    """
+    expected = 'mock config from default position'
+
+    _assert_about_get_egrc_config(
+        cli_path=None,
+        cli_config_exists=False,
+        winning_config_path=os.path.join('path', 'to', 'xdg_home', 'eg',
+                                         'egrc_expanded'),
+        home_config_exists=True,
+        expected_config=expected,
+        xdg_dir_variable=os.path.join('path', 'to', 'xdg_home'),
+        xdg_config_exists=True,
     )
 
 
@@ -200,39 +243,63 @@ def test_get_egrc_returns_empty_if_no_egrc():
 
     _assert_about_get_egrc_config(
         cli_path=None,
-        path_to_expand=config.DEFAULT_EGRC_PATH,
+        cli_config_exists=False,
+        winning_config_path=None,
+        home_config_exists=False,
         expected_config=expected,
-        is_file=False,
+        xdg_dir_variable=os.path.join('path', 'to', 'xdg_home'),
+        xdg_config_exists=False,
     )
 
 
 @patch('eg.config.get_config_tuple_from_egrc')
 @patch('eg.config.get_expanded_path')
 @patch('os.path.isfile')
+@patch('os.getenv')
 def _assert_about_get_egrc_config(
+    mock_getenv,
     mock_isfile,
     mock_expand,
     mock_get_config,
     cli_path=None,
-    path_to_expand=None,
-    is_file=True,
-    expected_config=None
+    winning_config_path=None,
+    cli_config_exists=False,
+    home_config_exists=False,
+    xdg_config_exists=False,
+    xdg_dir_variable=None,
+    expected_config=None,
 ):
-    expanded_path = path_to_expand + 'expanded'
+    def expand_side_effect(file_name):
+      return file_name + '_expanded'
+    mock_expand.side_effect = expand_side_effect
+
+    def isfile_side_effect(file_name):
+      if cli_path and file_name == cli_path + '_expanded':
+        return cli_config_exists
+      if file_name == os.path.join('~', '.egrc_expanded'):
+        return home_config_exists
+      if file_name == os.path.join(xdg_dir_variable, 'eg', 'egrc_expanded'):
+        # ${XDG_DIR_HOME}/eg/egrc
+        return xdg_config_exists
+      return False
+    mock_isfile.side_effect = isfile_side_effect
+
+    def getenv_side_effect(var):
+      if var == 'XDG_CONFIG_HOME':
+        return xdg_dir_variable
+      return None
+    mock_getenv.side_effect = getenv_side_effect
 
-    mock_isfile.return_value = is_file
-    mock_expand.return_value = expanded_path
     mock_get_config.return_value = expected_config
 
     actual = config.get_egrc_config(cli_path)
 
     assert actual == expected_config
 
-    mock_expand.assert_called_once_with(path_to_expand)
-    mock_isfile.assert_called_once_with(expanded_path)
-
-    if (is_file):
-        mock_get_config.assert_called_once_with(expanded_path)
+    if (winning_config_path):
+        mock_get_config.assert_called_once_with(winning_config_path)
+    else:
+        mock_get_config.assert_not_called()
 
 
 @patch('eg.config.get_expanded_path')