From 604616050b845d5b0bb858f634e3c19ab295e18d Mon Sep 17 00:00:00 2001
From: David Logie <djl@mksh.org>
Date: Sat, 26 Jan 2019 11:17:17 +0000
Subject: [PATCH] Address PR comments.

- Rename config option to overwrite_null
- Leave overwrite_null empty by default
- Handle more potentially null fields for both album and tracks
- Remove documentation and changelog entries for now
---
 beets/autotag/__init__.py | 68 ++++++++++++++++++++++-----------------
 beets/config_default.yaml | 16 ++-------
 docs/changelog.rst        |  3 --
 docs/reference/config.rst | 24 --------------
 4 files changed, 41 insertions(+), 70 deletions(-)

diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py
index f0c73d3c0..bf347b1e0 100644
--- a/beets/autotag/__init__.py
+++ b/beets/autotag/__init__.py
@@ -142,39 +142,47 @@ def apply_metadata(album_info, mapping):
         # Compilation flag.
         item.comp = album_info.va
 
-        # Miscellaneous metadata.
-        for field in ('albumtype',
-                      'label',
-                      'asin',
-                      'catalognum',
-                      'script',
-                      'language',
-                      'country',
-                      'albumstatus',
-                      'albumdisambig',
-                      'releasegroupdisambig',
-                      'data_source',):
-            # Don't overwrite fields with empty values unless the
-            # field is explicitly allowed to be overwritten
-            clobber = field not in config['no_clobber'].as_str_seq()
+        # Track alt.
+        item.track_alt = track_info.track_alt
+
+        # Miscellaneous/nullable metadata.
+        misc_fields = {
+            'album': (
+                'albumtype',
+                'label',
+                'asin',
+                'catalognum',
+                'script',
+                'language',
+                'country',
+                'albumstatus',
+                'albumdisambig',
+                'releasegroupdisambig',
+                'data_source',
+            ),
+            'track': (
+                'disctitle',
+                'lyricist',
+                'media',
+                'composer',
+                'composer_sort',
+                'arranger',
+            )
+        }
+
+        # Don't overwrite fields with empty values unless the
+        # field is explicitly allowed to be overwritten
+        for field in misc_fields['album']:
+            clobber = field in config['overwrite_null']['album'].as_str_seq()
             value = getattr(album_info, field)
             if value is None and not clobber:
                 continue
             item[field] = value
 
-        if track_info.disctitle is not None:
-            item.disctitle = track_info.disctitle
+        for field in misc_fields['track']:
+            clobber = field in config['overwrite_null']['track'].as_str_seq()
+            value = getattr(track_info, field)
+            if value is None and not clobber:
+                continue
+            item[field] = value
 
-        if track_info.media is not None:
-            item.media = track_info.media
-
-        if track_info.lyricist is not None:
-            item.lyricist = track_info.lyricist
-        if track_info.composer is not None:
-            item.composer = track_info.composer
-        if track_info.composer_sort is not None:
-            item.composer_sort = track_info.composer_sort
-        if track_info.arranger is not None:
-            item.arranger = track_info.arranger
-
-        item.track_alt = track_info.track_alt
diff --git a/beets/config_default.yaml b/beets/config_default.yaml
index 0e6e01efa..cf9ae6bf9 100644
--- a/beets/config_default.yaml
+++ b/beets/config_default.yaml
@@ -30,19 +30,6 @@ import:
     bell: no
     set_fields: {}
 
-no_clobber:
-  - albumtype
-  - label
-  - asin
-  - catalognum
-  - script
-  - language
-  - country
-  - albumstatus
-  - albumdisambig
-  - releasegroupdisambig
-  - data_source
-
 clutter: ["Thumbs.DB", ".DS_Store"]
 ignore: [".*", "*~", "System Volume Information", "lost+found"]
 ignore_hidden: yes
@@ -66,6 +53,9 @@ aunique:
     disambiguators: albumtype year label catalognum albumdisambig releasegroupdisambig
     bracket: '[]'
 
+overwrite_null:
+  album: []
+  track: []
 
 plugins: []
 pluginpath: []
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 746a3f5be..b896604c2 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -63,9 +63,6 @@ New features:
   provider: you can match tracks and albums using the Spotify database.
   Thanks to :user:`rhlahuja`.
   :bug:`3123`
-* A new ``no_clobber`` configuration option allows setting a list of
-  fields not to be overwritten by empty values upon re-importing items.
-  :bug:`3132`
 
 
 Changes:
diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index 44f7dd576..684dea20c 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -304,30 +304,6 @@ The defaults look like this::
 See :ref:`aunique` for more details.
 
 
-.. _no_clobber:
-
-no_clobber
-~~~~~~~~~~
-
-A list of fields that should not be overwritten by empty values when
-re-importing items.
-
-The default is::
-
-    no_clobber:
-      - albumtype
-      - label
-      - asin
-      - catalognum
-      - script
-      - language
-      - country
-      - albumstatus
-      - albumdisambig
-      - releasegroupdisambig
-      - data_source
-
-
 .. _terminal_encoding:
 
 terminal_encoding