Download & Extend

Windows directory separator compatibility

Project:API
Version:master
Component:Code
Category:bug report
Priority:critical
Assigned:Unassigned
Status:closed (works as designed)

Issue Summary

api.module currently does not work on windows because it relies on linux path conventions (':' as dir separator, absolute paths starting with '/') only. attached patch fixes this in a transparent manner. please review, test, and apply. thanks!

AttachmentSize
api.patch3.99 KB

Comments

#1

Title:windows compatibility» better performance, windows compatibility, exclude patterns

* don't loop through all files in all dirs twice (once for reading, once for removing $common_ancestor), but use a custom file_scan_directory() which does this at once
* check for allowed file extensions earlier, don't process those not allowed any further
* now using PATH_SEPARATOR for portability
* (inspired by Doxygen) added 'exclude_pattern', a preg expression of files and dirs matched against the absolute path not to be indexed. great for those test and contrib and symlinked files and dirs lingering around. including update file.

this is working great for me. if anyone else finds it useful, the better.

AttachmentSize
perf-exclude-win.patch 10.72 KB

#2

missed a case.

would anyone mind to review this? thanks!

AttachmentSize
perf-exclude-win.patch 10.52 KB

#3

Status:needs review» needs work

I ran into the same problem with the API module not working quite right on windows, and applied this patch. It did solve my problem in that respect, but I haven't given the code itself or the other features added by the patch a review.

I would advise breaking this up into several issues, since lumping several improvements/bug fixes together is generally a bad idea.

I would have three issues:
1. bug report for windows path separators
2. feature request for exclude patterns
3. feature request/bug report for performance improvements

#4

Version:5.x-1.x-dev» 5.x-1.0
Priority:normal» critical
Status:needs work» reviewed & tested by the community

Hi,

You can't really unlump this, as it all goes into the same api_scan_directories() function.

The closer you can come is to create a patch for the path separator and another for the exclude/performance.

Anyway, I was having the problem that the API module was choking up on the PDF libraries for the print module and with this patch I can now confirm that the exclude pattern is working properly.

So, with aclight having tested it on Windows for the path separator and me testing it in Linux for the exclude patterns and both of us having of course tested that the performance improvements don't break anything, i think it's ready to be committed.

João

#5

Status:reviewed & tested by the community» needs work

Yes, this must be un-lumped and the code does not apply after many recent improvements. This should only tackle directorey separator issues. The : path separator has been changed to a newline, because one path per line is much easier to read than everything mashed together with an arbitrary character in the middle.

Excluding some files is an interesting idea. I am not quite sure what the use case is, it is not a priority for me. If it is implemented as a separate feature request, then it should probably work like the block module's configuration, one basic path pattern per-line. I do not think end users should ever have to read a regular expression, even if this is a highly-technical module.

Improvements to the common_ancestor stuff are welcome regardless, it is hard to figure out what is going on by reading the uncommented code.

#6

Title:better performance, windows compatibility, exclude patterns» Windows directory separator compatibility

#7

Version:5.x-1.0» master

#8

Status:needs work» closed (works as designed)

This is fully obsoleted by other changes.