From a8de97c32719792980f7c07776a49d7535a70cfd Mon Sep 17 00:00:00 2001 From: Olivier Mangin Date: Thu, 9 Aug 2018 19:00:01 +0200 Subject: [PATCH] Fixes #144: behavior of add_copy mode during add. - correctly handles add_copy mode and configuration, - add option top force 'copy' mode (since the default had been changed in between from 'copy' to 'move' this is now needed), - fixes assumption in one test that the default is 'copy' (in other words the test was broken and ensuring that the functionality was broken too.), - do not try to delete the source when it is an URL. --- pubs/commands/add_cmd.py | 42 +++++++++++++++------------ tests/test_usecase.py | 63 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 22 deletions(-) diff --git a/pubs/commands/add_cmd.py b/pubs/commands/add_cmd.py index d193b5e..9487d05 100644 --- a/pubs/commands/add_cmd.py +++ b/pubs/commands/add_cmd.py @@ -34,10 +34,19 @@ def parser(subparsers, conf): ).completer = CommaSeparatedTagsCompletion(conf) parser.add_argument('-k', '--citekey', help='citekey associated with the paper;\nif not provided, one will be generated automatically.', default=None, type=p3.u_maybe) - parser.add_argument('-L', '--link', action='store_false', dest='copy', default=True, - help="don't copy document files, just create a link.") - parser.add_argument('-M', '--move', action='store_true', dest='move', default=False, - help="move document instead of of copying (ignored if --link).") + doc_add_group = parser.add_mutually_exclusive_group() + doc_add_group.add_argument( + '-L', '--link', action='store_const', dest='doc_add', const='link', + default=None, + help="don't copy document files, just create a link.") + doc_add_group.add_argument( + '-M', '--move', action='store_const', dest='doc_add', const='move', + default=None, + help="move document instead of of copying.") + doc_add_group.add_argument( + '-C', '--copy', action='store_const', dest='doc_add', const='copy', + default=None, + help="copy document instead of of move.") return parser @@ -136,25 +145,20 @@ def command(conf, args): '{}, using {} instead.').format(bib_docfile, docfile)) # create the paper - copy = args.copy - if copy is None: - copy = conf['main']['doc_add'] in ('copy', 'move') - move = args.move - if move is None: - move = conf['main']['doc_add'] == 'move' + doc_add = args.doc_add + if doc_add is None: + doc_add = conf['main']['doc_add'] rp.push_paper(p) ui.message('added to pubs:\n{}'.format(pretty.paper_oneliner(p))) if docfile is not None: - rp.push_doc(p.citekey, docfile, copy=copy or args.move) - if copy: - if move: - content.remove_file(docfile) - - if copy: - if move: - ui.message('{} was moved to the pubs repository.'.format(docfile)) - else: + rp.push_doc(p.citekey, docfile, copy=(doc_add in ('copy', 'move'))) + if doc_add == 'move' and content.content_type(docfile) != 'url': + content.remove_file(docfile) + + if doc_add == 'move': + ui.message('{} was moved to the pubs repository.'.format(docfile)) + elif doc_add == 'copy': ui.message('{} was copied to the pubs repository.'.format(docfile)) rp.close() diff --git a/tests/test_usecase.py b/tests/test_usecase.py index 43cbf2c..fb8a109 100644 --- a/tests/test_usecase.py +++ b/tests/test_usecase.py @@ -292,9 +292,66 @@ class TestAdd(URLContentTestCase): 'pubs add data/pagerank.bib --link -d data/pagerank.pdf', ] self.execute_cmds(cmds) - self.assertEqual(os.listdir( - os.path.join(self.default_pubs_dir, 'doc')), + self.assertEqual( + os.listdir(os.path.join(self.default_pubs_dir, 'doc')), []) + self.assertTrue(os.path.exists('data/pagerank.pdf')) + + def test_add_doc_nocopy_from_config_does_not_copy(self): + self.execute_cmds(['pubs init']) + config = conf.load_conf() + config['main']['doc_add'] = 'link' + conf.save_conf(config) + cmds = ['pubs add data/pagerank.bib -d data/pagerank.pdf'] + self.execute_cmds(cmds) + self.assertEqual( + os.listdir(os.path.join(self.default_pubs_dir, 'doc')), + []) + self.assertTrue(os.path.exists('data/pagerank.pdf')) + + def test_add_doc_copy(self): + cmds = ['pubs init', + 'pubs add data/pagerank.bib --copy -d data/pagerank.pdf', + ] + self.execute_cmds(cmds) + self.assertEqual( + os.listdir(os.path.join(self.default_pubs_dir, 'doc')), + ['Page99.pdf']) + self.assertTrue(os.path.exists('data/pagerank.pdf')) + + def test_add_doc_copy_from_config(self): + self.execute_cmds(['pubs init']) + config = conf.load_conf() + config['main']['doc_add'] = 'copy' + conf.save_conf(config) + cmds = ['pubs add data/pagerank.bib -d data/pagerank.pdf'] + self.execute_cmds(cmds) + self.assertEqual( + os.listdir(os.path.join(self.default_pubs_dir, 'doc')), + ['Page99.pdf']) + self.assertTrue(os.path.exists('data/pagerank.pdf')) + + def test_add_doc_move(self): + cmds = ['pubs init', + 'pubs add data/pagerank.bib --move -d data/pagerank.pdf', + ] + self.execute_cmds(cmds) + self.assertEqual( + os.listdir(os.path.join(self.default_pubs_dir, 'doc')), + ['Page99.pdf']) + self.assertFalse(os.path.exists('data/pagerank.pdf')) + + def test_add_doc_move_from_config(self): + self.execute_cmds(['pubs init']) + config = conf.load_conf() + config['main']['doc_add'] = 'move' + conf.save_conf(config) + cmds = ['pubs add data/pagerank.bib -d data/pagerank.pdf'] + self.execute_cmds(cmds) + self.assertEqual( + os.listdir(os.path.join(self.default_pubs_dir, 'doc')), + ['Page99.pdf']) + self.assertFalse(os.path.exists('data/pagerank.pdf')) def test_add_move_removes_doc(self): cmds = ['pubs init', @@ -611,7 +668,7 @@ class TestUsecase(DataCommandTestCase): def test_first(self): correct = ['Initializing pubs in /paper_first\n', 'added to pubs:\n[Page99] Page, Lawrence et al. "The PageRank Citation Ranking: Bringing Order to the Web." (1999) \n' - 'data/pagerank.pdf was copied to the pubs repository.\n', + 'data/pagerank.pdf was moved to the pubs repository.\n', '[Page99] Page, Lawrence et al. "The PageRank Citation Ranking: Bringing Order to the Web." (1999) \n', '\n', '',