safer install: interactive install, --edit and --force option

If existing install is found, prompt user whether to edit existing
install (keep previous defaults and update only new info such as bibtex
or --local/--global) or to overwrite it. To avoid the prompt, --edit and
--force options are available.

Also added install tests to check for --edit / --force behaviour

papers/__main__.py

@@ -61,18 +61,26 @@ def installcmd(parser, o, config):
  """
  Given options and a config state, installs the expected config files.
  """
- if o.reset_paths:
+ prompt = o.prompt and not o.edit
+ # installed = config.file is not None
+ if config.file is not None and prompt:
+ ans = input('An existing install was found: {config.file}. Overwrite (O) or Edit (E) ? [o / e]')
+ if ans.lower() not in ('o', 'e'):
+ parser.error('Use the --edit option to selectively edit existing configuration, or --force to ignore pre-existing configuration.')
+ o.edit = ans.lower() == 'e'
+
+ if not o.edit:
  config = Config()
 
  set_nameformat_config_from_cmd(o, config)
  set_keyformat_config_from_cmd(o, config)
 
  checkdirs = ["files", "pdfs", "pdf", "papers", "bibliography"]
- default_bibtex = "papers.bib"
- default_filesdir = "files"
+ default_bibtex = config.bibtex or "papers.bib"
+ default_filesdir = config.filesdir or "files"
 
  if o.local:
- papersconfig = ".papers/config.json"
+ papersconfig = config.file or ".papers/config.json"
  workdir = Path('.')
  bibtex_files = list(workdir.glob("*.bib"))
 
@@ -106,7 +114,7 @@ def installcmd(parser, o, config):
  logger.warn("Several bibtex files found: "+" ".join([str(b) for b in bibtex_files]))
  if bibtex_files:
  default_bibtex = bibtex_files[0]
- if o.prompt:
+ if prompt:
  if os.path.exists(default_bibtex):
  user_input = input(f"Bibtex file name [default to existing: {default_bibtex}] [Enter/Yes/No]: ")
  else:
@@ -121,7 +129,7 @@ def installcmd(parser, o, config):
  o.bibtex = default_bibtex
 
  if not o.filesdir:
- if o.prompt:
+ if prompt:
  if Path(default_filesdir).exists():
  user_input = input(f"Files folder [default to existing: {default_filesdir}] [Enter/Yes/No]: ")
  else:
@@ -539,8 +547,6 @@ def get_parser(config=None):
  help=f'bibtex database (default: {config.bibtex}')
  grp.add_argument('--dry-run', action='store_true',
  help='no PDF renaming/copying, no bibtex writing on disk (for testing)')
- grp.add_argument('--no-prompt', action='store_false', dest="prompt",
- help='no prompt, use default (useful for tests)')
  grp.add_argument('--relative-paths', action="store_false", dest="absolute_paths", default=None)
  grp.add_argument('--absolute-paths', action="store_true", default=None)
 
@@ -590,8 +596,10 @@ def get_parser(config=None):
 
  installp = subparsers.add_parser('install', description='setup or update papers install',
  parents=[cfg, namefmt, keyfmt])
- installp.add_argument('--reset-paths', action='store_true', help=argparse.SUPPRESS)
- # egrp = installp.add_mutually_exclusive_group()
+ installp.add_argument('--edit', action='store_true', help=f'edit existing install if any (found: {config.file})')
+ installp.add_argument('--force', '--no-prompt', action='store_false', dest="prompt",
+ help='no prompt, use default (useful for tests)')
+
  installp.add_argument('--local', action="store_true",
  help="""setup papers locally in current directory (global install by default), exposing bibtex and filesdir,
  and having the rest under .papers (config options). Only keep the cache globally.

tests/test_install.py

@@ -1,11 +1,14 @@
 import os
+import json
 import shutil
 import tempfile
 import unittest
 import subprocess as sp
+from papers.config import Config, search_config
+from papers.config import CONFIG_FILE
 # from pathlib import Path
 
-from tests.common import paperscmd, prepare_paper, run
+from tests.common import paperscmd, prepare_paper, run, PAPERSCMD
 
 bibtex = """@article{Perrette_2011,
  author = {M. Perrette and A. Yool and G. D. Quartly and E. E. Popova},
@@ -31,11 +34,10 @@ def setUp(self):
  open(os.path.join(self.temp_dir.name, self.anotherbib), 'w').write(bibtex)
 
  def tearDown(self):
- self.papers(f'uninstall')
  self.temp_dir.cleanup()
 
- # def _path(self, p):
- #  return os.path.join(self.temp_dir, p)
+ def _path(self, p):
+ return os.path.join(self.temp_dir.name, p)
 
  def _exists(self, p):
  return os.path.exists(os.path.join(self.temp_dir.name, p))
@@ -50,19 +52,99 @@ class TestLocalInstall(TestBaseInstall):
  def test_install(self):
  self.assertFalse(self._exists(self.mybib))
  self.assertFalse(self._exists(self.filesdir))
- self.papers(f'install --no-prompt --local --bibtex {self.mybib} --files {self.filesdir}')
+ self.papers(f'install --force --local --bibtex {self.mybib} --files {self.filesdir}')
  self.assertTrue(self._exists(self.mybib))
  self.assertTrue(self._exists(self.filesdir))
 
+ def test_install_raise(self):
+ self.papers(f'install --force --local --bibtex {self.mybib} --files {self.filesdir}')
+ self.assertTrue(self._exists(".papers/config.json"))
+ self.assertTrue(self._exists(self.mybib))
+ self.assertTrue(self._exists(self.filesdir))
+ f = lambda : self.papers(f'install --local --bibtex {self.mybib} --files {self.filesdir}')
+ self.assertRaises(Exception, f)
+
+ def test_install_force(self):
+ self.papers(f'install --force --local --bibtex {self.mybib} --files {self.filesdir}')
+ self.assertTrue(self._exists(".papers/config.json"))
+ self.assertTrue(self._exists(self.mybib))
+ self.assertTrue(self._exists(self.filesdir))
+ config = Config.load(self._path(".papers/config.json"))
+ self.assertEqual(config.bibtex, os.path.abspath(self._path(self.mybib)))
+ self.assertEqual(config.filesdir, os.path.abspath(self._path(self.filesdir)))
+ self.papers(f'install --local --force --bibtex {self.mybib}XX')
+ config = Config.load(self._path(".papers/config.json"))
+ self.assertEqual(config.bibtex, os.path.abspath(self._path(self.mybib + "XX")))
+ # The files folder from previous install was forgotten
+ self.assertEqual(config.filesdir, os.path.abspath(self._path("files")))
+
+ def test_install_edit(self):
+ self.papers(f'install --force --local --bibtex {self.mybib} --files {self.filesdir}')
+ self.assertTrue(self._exists(".papers/config.json"))
+ self.assertTrue(self._exists(self.mybib))
+ self.assertTrue(self._exists(self.filesdir))
+ config = Config.load(self._path(".papers/config.json"))
+ self.assertEqual(config.bibtex, os.path.abspath(self._path(self.mybib)))
+ self.assertEqual(config.filesdir, os.path.abspath(self._path(self.filesdir)))
+ self.papers(f'install --local --edit --bibtex {self.mybib}XX')
+ config = Config.load(self._path(".papers/config.json"))
+ self.assertEqual(config.bibtex, os.path.abspath(self._path(self.mybib + "XX")))
+ # The files folder from previous install is remembered
+ self.assertEqual(config.filesdir, os.path.abspath(self._path(self.filesdir)))
+
+ def test_install_interactive(self):
+ self.papers(f'install --force --local --bibtex {self.mybib} --files {self.filesdir}')
+ self.assertTrue(self._exists(".papers/config.json"))
+ self.assertTrue(self._exists(self.mybib))
+ self.assertTrue(self._exists(self.filesdir))
+ config = Config.load(self._path(".papers/config.json"))
+ self.assertEqual(config.bibtex, os.path.abspath(self._path(self.mybib)))
+ self.assertEqual(config.filesdir, os.path.abspath(self._path(self.filesdir)))
+
+ sp.check_call(f"""{PAPERSCMD} install --local --bibtex {self.mybib}XX << EOF
+e
+y
+EOF""", shell=True, cwd=self.temp_dir.name)
+ config = Config.load(self._path(".papers/config.json"))
+ self.assertEqual(config.bibtex, os.path.abspath(self._path(self.mybib + "XX")))
+ # The files folder from previous install is remembered
+ self.assertEqual(config.filesdir, os.path.abspath(self._path(self.filesdir)))
+
+ sp.check_call(f"""{PAPERSCMD} install --local --bibtex {self.mybib}XX << EOF
+o
+y
+EOF""", shell=True, cwd=self.temp_dir.name)
+ config = Config.load(self._path(".papers/config.json"))
+ self.assertEqual(config.bibtex, os.path.abspath(self._path(self.mybib + "XX")))
+ # The files folder from previous install was forgotten
+ self.assertEqual(config.filesdir, os.path.abspath(self._path("files")))
+
 
 class TestGlobalInstall(TestBaseInstall):
 
+ def setUp(self):
+ if os.path.exists(CONFIG_FILE):
+ self.backup = tempfile.mktemp(prefix='papers.bib.backup')
+ shutil.move(CONFIG_FILE, self.backup)
+ else:
+ self.backup = None
+ super().setUp()
+
+
  def test_install(self):
  self.assertFalse(self._exists(self.mybib))
  self.assertFalse(self._exists(self.filesdir))
+ self.assertFalse(os.path.exists(CONFIG_FILE))
  self.papers(f'install --no-prompt --bibtex {self.mybib} --files {self.filesdir}')
  self.assertTrue(self._exists(self.mybib))
  self.assertTrue(self._exists(self.filesdir))
+ self.assertTrue(os.path.exists(CONFIG_FILE))
+
+ def tearDown(self):
+ super().tearDown()
+ os.remove(CONFIG_FILE)
+ if self.backup:
+ shutil.move(self.backup, CONFIG_FILE)
 
 
 class TestGitInstall(TestBaseInstall):