diff --git a/changelog.md b/changelog.md index 5e07c14..c9ea407 100644 --- a/changelog.md +++ b/changelog.md @@ -8,6 +8,8 @@ ### Implemented enhancements +- Adds `move`, and `link` options for handling of documents during `import` (copy being the default). Makes `copy` the default for document handling during `add`. [(#159)](https://github.com/pubs/pubs/pull/159) + - Support for downloading arXiv reference from their ID ([#146](https://github.com/pubs/pubs/issues/146) by [joe-antognini](https://github.com/joe-antognini)) - Better feedback when an error is encountered while adding a reference from a DOI, ISBN or arXiv ID [#155](https://github.com/pubs/pubs/issues/155) @@ -34,6 +36,8 @@ ### Fixed bugs +- [[#144]](https://github.com/pubs/pubs/issues/144) More robust handling of the `doc_add` options [(#159)](https://github.com/pubs/pubs/pull/159) + - [[#149]](https://github.com/pubs/pubs/issues/149) More robust handling of parsing and citekey errors [(#87)](https://github.com/pubs/pubs/pull/87) - [[#148]](https://github.com/pubs/pubs/issues/148) Fix compatibility with Pyfakefs 3.7 [(#151)](https://github.com/pubs/pubs/pull/151) diff --git a/pubs/command_utils.py b/pubs/command_utils.py new file mode 100644 index 0000000..0446824 --- /dev/null +++ b/pubs/command_utils.py @@ -0,0 +1,18 @@ +"""Contains code that is reused over commands, like argument definition +or help messages. +""" + + +def add_doc_copy_arguments(parser, copy=True): + doc_add_group = parser.add_mutually_exclusive_group() + doc_add_group.add_argument( + '-L', '--link', action='store_const', dest='doc_copy', const='link', + default=None, + help="don't copy document files, just create a link.") + if copy: + doc_add_group.add_argument( + '-C', '--copy', action='store_const', dest='doc_copy', const='copy', + help="copy document (keep source).") + doc_add_group.add_argument( + '-M', '--move', action='store_const', dest='doc_copy', const='move', + help="move document (copy and remove source).") diff --git a/pubs/commands/add_cmd.py b/pubs/commands/add_cmd.py index dead1a7..d02e06e 100644 --- a/pubs/commands/add_cmd.py +++ b/pubs/commands/add_cmd.py @@ -12,6 +12,7 @@ from .. import apis from .. import pretty from .. import utils from .. import endecoder +from ..command_utils import add_doc_copy_arguments from ..completion import CommaSeparatedTagsCompletion @@ -36,10 +37,7 @@ 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).") + add_doc_copy_arguments(parser) return parser @@ -140,25 +138,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_copy + 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/pubs/commands/import_cmd.py b/pubs/commands/import_cmd.py index fa6236e..7cbf3b6 100644 --- a/pubs/commands/import_cmd.py +++ b/pubs/commands/import_cmd.py @@ -7,10 +7,11 @@ from .. import repo from .. import endecoder from .. import bibstruct from .. import color +from .. import content from ..paper import Paper - from ..uis import get_ui from ..content import system_path, read_text_file +from ..command_utils import add_doc_copy_arguments _ABORT_USE_IGNORE_MSG = "Aborting import. Use --ignore-malformed to ignore." @@ -18,18 +19,22 @@ _IGNORING_MSG = " Ignoring it." def parser(subparsers, conf): - parser = subparsers.add_parser('import', - help='import paper(s) to the repository') - parser.add_argument('bibpath', - help='path to bibtex, bibtexml or bibyaml file (or directory)') - 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('keys', nargs='*', - help="one or several keys to import from the file") - parser.add_argument('-O', '--overwrite', action='store_true', default=False, - help="Overwrite keys already in the database") - parser.add_argument('-i', '--ignore-malformed', action='store_true', default=False, - help="Ignore malformed and unreadable files and entries") + parser = subparsers.add_parser( + 'import', + help='import paper(s) to the repository') + parser.add_argument( + 'bibpath', + help='path to bibtex, bibtexml or bibyaml file (or directory)') + parser.add_argument( + 'keys', nargs='*', + help="one or several keys to import from the file") + parser.add_argument( + '-O', '--overwrite', action='store_true', default=False, + help="Overwrite keys already in the database") + parser.add_argument( + '-i', '--ignore-malformed', action='store_true', default=False, + help="Ignore malformed and unreadable files and entries") + add_doc_copy_arguments(parser, copy=False) return parser @@ -90,9 +95,7 @@ def command(conf, args): ui = get_ui() bibpath = args.bibpath - copy = args.copy - if copy is None: - copy = conf['main']['doc_add'] in ('copy', 'move') + doc_import = args.doc_copy or 'copy' rp = repo.Repository(conf) # Extract papers from bib @@ -106,7 +109,9 @@ def command(conf, args): if docfile is None: ui.warning("No file for {}.".format(p.citekey)) else: - rp.push_doc(p.citekey, docfile, copy=copy) - # FIXME should move the file if configured to do so. + rp.push_doc(p.citekey, docfile, + copy=(doc_import in ('copy', 'move'))) + if doc_import == 'move' and content.content_type(docfile) != 'url': + content.remove_file(docfile) rp.close() diff --git a/pubs/config/spec.py b/pubs/config/spec.py index d881898..76421a4 100644 --- a/pubs/config/spec.py +++ b/pubs/config/spec.py @@ -11,7 +11,7 @@ docsdir = string(default="docsdir://") # Specify if a document should be copied or moved in the docdir, or only # linked when adding a publication. -doc_add = option('copy', 'move', 'link', default='move') +doc_add = option('copy', 'move', 'link', default='copy') # the command to use when opening document files open_cmd = string(default=None) diff --git a/tests/test_usecase.py b/tests/test_usecase.py index 11cce7a..62ce138 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',