On Sat, Jul 29, 2006, EV wrote:
> Thanks for the patch!  I'm testing it now and it ooks fine:  
> nothing seems to be broken ;)
> 
> However, I don't really like the *change* of tr-edditing for
> escaping.  I think path escape-edditing can lead to ugly paths in
> some cases.  If the user (me ;) is confident enough about the
> reversibility of tr-edditing, he/she may (strongly) preffer tr
> than escape.  For instance, with 'lkarmafs -E "/ " "#_" mnt/' you
> get things like:
> 
>   mnt/tune/Jazz/Carmen\_McRae/Blue\_Moon/My\_Foolish\_Heart.mp3

It is the only way to *guarantee* reversibility. I never really saw the
point in arbitrarily changing characters in the pathname and especially
disliked the fact that this often led to unexpected behaviour. However,
I guess that if people want this functionality then it should be put
back in.
The escape code should definitely be used for the '/' character, though,
since this is necessary for correct operation of the filesystem. Maybe
just hard-code it in and make it non-optional?

> On the other hand, I don't quite understand the motivation for
> both escaping *and* substituting.  If you doubt of some critical
> char like ':' you may want to escape *it*; i.e., '\:', but not
> something like '\@'.

I figured that you are substituting letters because you don't want them
appearing in the final pathname. For the '/' character, for instance, it
would be totally pointless to replace it by '\/'. That would still wreak
havoc with the filesystem. You can still replace ':' by '\:' using the
option "-E ':' ':'"

> So I think escape-edditing should be optional or additional to
> tr-edditing.  For the moment I'm keeping lkarmafs-0.1.7 as the
> current tree.  If you want to send in another patch, please do it
> against 0.1.7.  Otherwise, I'll take some other parts of your
> patch (like codec = strrchr(title, '.')) and I'll try myself to
> add optional escaping.

There is one method of doing this which might be a possible compromise.
When it comes to swapping characters you only need to escape one set:
either the originals or the replacements. For example, if you are replacing
the ' ' character with '_' then you are fine until you come across a
pathname that already contains the '_' character. Then you need some way
of telling the original instances of '_' from the ones that have just been
introduced. There are two ways of dealing with this - either escape the
originals or escape the replacements. I chose to escape the replacements
but you could just as easily escape the originals instead.

Here are two examples of replacing ' ' with '_'. The first is the current
method. The second escapes the originals instead.

  'A long pathname using_spaces_and_undercores'
   1 => 'A\_long\_pathname\_using_spaces_and_underscores'
   2 => 'A_long_pathname_using\_spaces\_and\_underscores'

If you prefer method two then let me know and I'll revise the patch.
Or if you would rather have the potentially dangerous tr-editing back
then I'll hard-code the '/' character translation. I think that this
is the only character that absolutely has to be changed?

> > I might be missing something, but the testing for reps part
> > seems a bit off. aux has the codec appended incorrectly, I
> > think. Not sure that the aux string is necessary at all for
> > this part. I'll take another look tomorrow.
> 
> As far as I remember, aux (and the wired codec appending) became
> necessary to change the RK codec value "vorbis" for "ogg".  
> Nevertheless, I'll take another look tomorrow to see if I find
> some bug (even though the testing doesn't reveal anything wrong).

One version appears to get a '.' and the other doesn't. This would only
be noticed if two filenames with the same pathname but different
encodings were used. If the rep list was null-terminated instead of
terminated by '\n' then the aux string becomes redundant and can be
removed altogether.

Keith.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-karma-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-karma-devel

Reply via email to