[MERGE] The eol functionality now issues a warning if a rule is configured (bug 358199)

[Brian de Alwis]

Thanks for the review.

I wonder now if a warning is the right behaviour; I now believe that  
bzr should actually throw an error on an unknown setting.  Otherwise  
the user might have different content stored in the repository than  
she might have desired.  When using a checkout or bound branch, such a  
commit { can't | likely shouldn't } be uncommitted.

Brian.

On 17-Apr-2009, at 1:11 PM, John Arbash Meinel wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Brian de Alwis wrote:
>> Attached is a patch to bzr.dev to emit a warning when encountering an
>> unknown eol setting.  This patch fixed bug 358199.
>>
>> Brian.
>>
>
> +	filter = _eol_filter_stack_map.get(key)
> +	if filter == None:
> +	     from bzrlib.trace import warning
> +	     warning("Unknown eol filter type '%s'" % key)
>
> ^- as a 'pythonic' thing, you should always use 'is' to compare with
> None, so it would be:
>
> if filter is None:
>  ...
>
> I'm also a bit concerned about filling the screens with warnings.
>
> Consider someone who does something like:
>
> [name *.c]
> eol = crlff
>
> At that point when the do 'bzr co' it will give 10,000 warnings about
> unknown eol filter type (I believe).
>
> So I think we would want a 'debounce' in it with something like:
>
> _unknown_eol = set()
>
> def eol_lookup(key):
>  ...
>  if filter is None and key not in _unknown_eol:
>    _unknown_eol.add(key)
>    warning("Unknown eol filter type '%s'", key)
>
> (note that 'warning' already has the right code to do % *args, so you
> can just pass 'key' as another argument.)
>
>
> ...
>
> +class TestEolRulesSpecifications(TestCase):
> +
> +    def setUp(self):
> +        self.warnings = []
> +        self._warning = trace.warning
> +	def warning(*args):
> +	    self.warnings.append(args[0] % args[1:])
> +        trace.warning = warning
> +	super(TestEolRulesSpecifications, self).setUp()
>
> ^- The spacing on this shows that you have some 'tab' characters mixed
> with plain space characters. We only allow plain space characters in  
> our
> python source code.
>
>
> I'm not 100% sure about adding a debounce to 'unknown eol', but I'm
> guessing that scrolling by 1000s of messages is undesirable.
>
> Anyway, because of tabs in the source:
> BB:resubmit
>
> for now. I think the change is otherwise reasonable.
>
> John
> =:->
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (Cygwin)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iEYEARECAAYFAkno1F0ACgkQJdeBCYSNAAN5rACgqUPFTeORS7v97HQhDMus9HO8
> wSwAoJTW5Q3zKEsMhEs8/bFiiiXpXx5R
> =H65n
> -----END PGP SIGNATURE-----

-- 
"Amusement to an observing mind is study." - Benjamin Disraeli