View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0024139||Lazarus||Patch||public||2013-03-26 14:15||2013-04-06 14:51|
|Reporter||ChrisF||Assigned To||Jesus Reyes|
|Product Version||Product Build|
|Target Version||1.2.0||Fixed in Version||1.1 (SVN)|
|Summary||0024139: PATCH for LAZRES: remove leading and trailing spaces for filenames in filelist|
A proposal for a very simple change to remove all leading and trailing spaces for file names, when using a external file list (i.e. '@filelist' argument) for Lazres.
Though space characters are allowed for file names in most of the current OS, having in fine a resource called ' MyResource' and/or a resource type 'MyType ' into a Lazarus resource file will be probably unintended in most cases (well, at least in my case, as I've done the mistake).
Plus, a resource file in filelist with ' MyResource.MyType' as a name (for instance) won't be found if it's real name is in fact 'MyResource.MyType' (i.e. without a leading space). And it's not evident in this error message 'ERROR: file not found: MyResource.MyType', that it doesn't mean that 'MyResource.MyType' is missing, but instead that ' MyResource.MyType' is; just because there are 2 space characters in this part of the error message'...found: MyResource...'.
Just my feelings. Please discard, if inappropriate.
|Tags||No tags attached.|
|Fixed in Revision||40675, 40685|
patch.zip (381 bytes)
||As in revision 40662 customizable resource names were introduced, your patch wouldn't work all times. In r40675 I tried to fix the issue, please test.|
After a few tests with this new version (r40683), there is still a problem with leading spaces (i.e. one or more space characters before the name of the file).
Anyway, I can't see in which cases the proposed patch "wouldn't work", even after the new parameters option. It's just a preliminary "space trimming" of all the lines present into the file list (which is just an ordinary text file, BTW).
Exactly as if the parameters were coming directly from the command line. I mean, you can't have space(s) before or after a file name (unless you quote it), when using a "direct" command, like:
lazres resourcefilename filename1[=resname1] [filename2[=resname2] ... filenameN=resname[N]]
Because leading and trailing spaces have already been removed when accessing to ParamStrUTF8(a).
I've made various tests with the current version of lazres (r40683 - so including your own modification, if I'm right) with the concerned patch, and couldn't find any errors.
Please, could you precise the possible wrong cases that you were referring to, with this patch ?
Attached the same patch but for the r40603, this time (i.e. patch2.diff).
patch2.diff (568 bytes)
Index: tools/lazres.pp =================================================================== --- tools/lazres.pp (r�vision 40683) +++ tools/lazres.pp (copie de travail) @@ -82,9 +82,11 @@ exit; end; FileList.LoadFromFile(UTF8ToSys(S)); - for a:=FileList.Count-1 downto 0 do + for a:=FileList.Count-1 downto 0 do begin + FileList[a]:=Trim(FileList[a]); if FileList[a]='' then FileList.Delete(a); + end; end else for a:=2 to ParamCount do FileList.Add(ParamStrUTF8(a)); // cleanup lines
patch2.diff (568 bytes)
With the new feature ([=ResName]) the proposed patch wouldn't work (or better, would work the wrong way you initally intented to fix) in this cases:
Filename = ResName
I only checked trailing space
Buf failed to check leading space as you noted, should be now fixed, please test.
||damn mantis do not allow to edit notes after issue was resolved|
According to my new tests, everything seems now fine. So please, close this report.
|2013-03-26 14:15||ChrisF||New Issue|
|2013-03-26 14:15||ChrisF||File Added: patch.zip|
|2013-04-01 05:16||Jesus Reyes||Assigned To||=> Jesus Reyes|
|2013-04-01 05:16||Jesus Reyes||Status||new => assigned|
|2013-04-01 05:20||Jesus Reyes||Fixed in Revision||=> 40675|
|2013-04-01 05:20||Jesus Reyes||LazTarget||=> 1.2|
|2013-04-01 05:20||Jesus Reyes||Note Added: 0066674|
|2013-04-01 05:20||Jesus Reyes||Status||assigned => resolved|
|2013-04-01 05:20||Jesus Reyes||Fixed in Version||=> 1.1 (SVN)|
|2013-04-01 05:20||Jesus Reyes||Resolution||open => fixed|
|2013-04-01 05:20||Jesus Reyes||Target Version||=> 1.2.0|
|2013-04-01 22:28||ChrisF||Note Added: 0066701|
|2013-04-01 22:28||ChrisF||File Added: patch2.diff|
|2013-04-01 22:28||ChrisF||Status||resolved => assigned|
|2013-04-01 22:28||ChrisF||Resolution||fixed => reopened|
|2013-04-01 22:59||Jesus Reyes||Fixed in Revision||40675 => 40675, 40685|
|2013-04-01 22:59||Jesus Reyes||Note Added: 0066702|
|2013-04-01 22:59||Jesus Reyes||Status||assigned => resolved|
|2013-04-01 22:59||Jesus Reyes||Resolution||reopened => fixed|
|2013-04-01 23:02||Jesus Reyes||Note Added: 0066703|
|2013-04-01 23:02||Jesus Reyes||Status||resolved => assigned|
|2013-04-01 23:02||Jesus Reyes||Resolution||fixed => reopened|
|2013-04-01 23:02||Jesus Reyes||Note Edited: 0066702||View Revisions|
|2013-04-01 23:02||Jesus Reyes||Status||assigned => resolved|
|2013-04-01 23:02||Jesus Reyes||Resolution||reopened => fixed|
|2013-04-04 00:54||ChrisF||Note Added: 0066794|
|2013-04-04 00:55||ChrisF||Note Edited: 0066794||View Revisions|
|2013-04-06 14:51||ChrisF||Note Added: 0066830|
|2013-04-06 14:51||ChrisF||Status||resolved => closed|