[PATCH] repository: Base: apply local->remote nametrans after sep-conversion

[Nicolas Sebrecht]

[ Sorry, I couldn't send this mail at the time I wrote it. ]

On Sat, Dec 24, 2016 at 11:15:58AM -0800, Andrej Rode wrote:

> Allow usage of nametrans to sync remote folders with local sep in their
> name.
> 
> Signed-off-by: Andrej Rode <mail at andrejro.de>
> ---
>  offlineimap/accounts.py        | 14 +++++++-------
>  offlineimap/repository/Base.py |  4 ++--
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
> index 327af8b..3e114e8 100644
> --- a/offlineimap/accounts.py
> +++ b/offlineimap/accounts.py
> @@ -361,13 +361,13 @@ class SyncableAccount(Account):
>                  # their names since this would cause troubles while
> converting
>                  # the name back (from local to remote).
>                  sep = localrepos.getsep()
> -                if (sep != os.path.sep and
> -                    sep != remoterepos.getsep() and
> -                    sep in remotefolder.getname()):
> -                    self.ui.warn('', "Ignoring folder '%s' due to
> unsupported "
> -                        "'%s' character serving as local separator."%
> -                        (remotefolder.getname(), localrepos.getsep()))
> -                    continue # Ignore unsupported folder name.
> +                # if (sep != os.path.sep and
> +                #     sep != remoterepos.getsep() and
> +                #     sep in remotefolder.getname()):
> +                #     self.ui.warn('', "Ignoring folder '%s' due to
> unsupported "
> +                #         "'%s' character serving as local separator."%
> +                #         (remotefolder.getname(), localrepos.getsep()))
> +                #     continue # Ignore unsupported folder name.

>                   localfolder = self.get_local_folder(remotefolder)
>                  if not localfolder.sync_this:
> diff --git a/offlineimap/repository/Base.py b/offlineimap/repository/Base.py
> index 0c3d16f..a38c111 100644
> --- a/offlineimap/repository/Base.py
> +++ b/offlineimap/repository/Base.py
> @@ -219,8 +219,8 @@ class BaseRepository(CustomConfig.ConfigHelperMixin):
>                  break
>               # Apply reverse nametrans and fix serparator.
> -            remote_name = local_folder.getvisiblename().replace(
> -                local_repo.getsep(), remote_repo.getsep())
> +            remote_name =
> local_folder.getrepository().nametrans(local_folder.getname().replace(

We limit the lines length to 72 chars.

> +                local_repo.getsep(), remote_repo.getsep()))

Some of the above commented checks would need to be moved here.

>              if local_folder.sync_this and not remote_name in
> remote_hash.keys():
>                  # Would the remote filter out the new folder name? In
> this case
>                  # don't create it.

I agree that we could be smarter than forbidding both local and remote
seps in the names.

However, I'm not conviced we should use nametrans to allow the users to
fix the sep issues. Perhaps introducing two new configuration options
would be better. Something like:

[remote side]
  convert_sep_to = ! # local sep becomes !

[local side]
  convert_sep_to = _ # remote sep becomes _

The sad news is that users will forget about this option and use
"wrongly converted" characters and make false positives. E.g., if the
user creates a remote folder name with !.

I think we must have correct checks once the names are converted.

-- 
Nicolas Sebrecht