On 03.07.2013 20:34, Jordan Rose wrote:
Comments on patch (which probably should have gone to cfe-commits or
Phabricator):
Attached the updated patch to http://llvm-reviews.chandlerc.com/D703
+# portability: convert POSIX cygwin path to MS-DOS style path
Please start comments with a capital letter.
+ $PortablePath =~ s/^\/cygdrive\/(\w)/$1:/o if ($^O =~ /cygwin/);
The user is free to modify the mount path for /cygdrive/, but I guess that's
okay. What I'm more confused about is how programs running inside Cygwin are
supposed to use Windows-ish paths (still with forward slashes) to run
executables. But I don't have a Windows machine available, so I'll trust you
know what's right here. Please do try to hand-test all the code paths affected
by this if you haven't already.
Tested with pure cygwin, windows paths are unwillingly accepted, with
warnings; cygwin docs says "Using native Win32 paths in Cygwin, while
possible, is generally inadvisable."
Decided to remove path conversion for now. This enhancement was added
for particular cases when cygwin is used together with other ports
(MinGW + perl from Cygwin + separate Python in my case).
+# portability: getpwuid is not implemented for Win32 (see Perl language
reference, perlport)
+my $UserName = ($^O =~/MSWin32/) ? HtmlEscape(getlogin || 'unknown')
+ : HtmlEscape(getpwuid($<) || 'unknown');
How about just getlogin() || getpwuid($<) || 'unknown'? That seems to be fairly
idiomatic in a quick search online.
(This is only used for metadata in the generated output, so it's not even that
important.)
+print "NewDir = ".$NewDir."\n";
Stray debug print?
+# portability: use less strict but portable check -e (file exists) instead of
+# non-portable -x (file is executable). On some windows ports -x just checks
+# file extension to determine if a file is executable (see Perl language
reference, perlport)
Capital letter, and we should also follow the 80-column limit going forward.
--
Anton
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
