Skip to content

Commit 14a1ba9

Browse files
committed
Tighten up Windows support and add a test
1 parent 96200cb commit 14a1ba9

File tree

3 files changed

+22
-7
lines changed

3 files changed

+22
-7
lines changed

lib/src/model/package_graph.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -951,18 +951,19 @@ class PackageGraph {
951951
allLocalModelElements.where((e) => e.isCanonical).toList();
952952
}
953953

954+
/// Glob lookups can be expensive. Cache per filename.
954955
final _configSetsNodocFor = HashMap<String, bool>();
955956

956957
/// Given an element's location, look up the nodoc configuration data and
957958
/// determine whether to unconditionally treat the element as "nodoc".
958-
/// Cached by filename to reduce repetitive lookups into the config data.
959959
bool configSetsNodocFor(String fullName) {
960960
if (!_configSetsNodocFor.containsKey(fullName)) {
961961
var file = resourceProvider.getFile(fullName);
962962
// Direct lookup instead of generating a custom context will save some
963963
// cycles. We can't use the element's [DartdocOptionContext] because that
964964
// might not be where the element was defined, which is what's important
965-
// for nodoc's semantics.
965+
// for nodoc's semantics. Looking up the defining element just to pull
966+
// a context is again, slow.
966967
List<String> globs = config.optionSet['nodoc'].valueAt(file.parent);
967968
_configSetsNodocFor[fullName] = matchGlobs(globs, fullName);
968969
}

lib/src/model_utils.dart

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'dart:io';
1010
import 'package:analyzer/dart/ast/ast.dart';
1111
import 'package:analyzer/dart/element/element.dart';
1212
import 'package:analyzer/src/dart/ast/utilities.dart';
13+
import 'package:dartdoc/dartdoc.dart';
1314
import 'package:dartdoc/src/model/model.dart';
1415
import 'package:path/path.dart' as path;
1516
import 'package:glob/glob.dart';
@@ -23,18 +24,27 @@ final Map<String, String> _fileContents = <String, String>{};
2324
/// Assumes that globs and resource provider are from the same drive, which
2425
/// will be the case for globs relative to dartdoc_options.yaml.
2526
///
26-
/// On windows, globs are assumed to use Windows paths in combination with
27-
/// globs, e.g. `C:\foo\bar\*.txt`.
27+
/// On windows, globs are assumed to use absolute Windows paths with drive
28+
/// letters in combination with globs, e.g. `C:\foo\bar\*.txt`. `fullName`
29+
/// also is assumed to have a drive letter.
2830
bool matchGlobs(List<String> globs, String fullName, {bool isWindows}) {
2931
isWindows ??= Platform.isWindows;
3032
var filteredGlobs = <String>[];
3133

3234
if (isWindows) {
33-
assert(_driveLetterMatcher.hasMatch(fullName),
34-
'can not find drive letter in $fullName');
35-
// TODO(jcollins-g): handle globs referencing different drives?
35+
// TODO(jcollins-g): port this special casing to the glob package.
36+
var fullNameDriveLetter = _driveLetterMatcher.stringMatch(fullName);
37+
if (fullNameDriveLetter == null) {
38+
throw DartdocFailure(
39+
'Unable to recognize drive letter on Windows in: $fullName');
40+
}
41+
// Build a matcher from the [fullName]'s drive letter to filter the globs.
42+
var driveGlob = RegExp(fullNameDriveLetter.replaceFirst(r'\', r'\\'),
43+
caseSensitive: false);
3644
fullName = fullName.replaceFirst(_driveLetterMatcher, r'\');
3745
for (var glob in globs) {
46+
// Globs don't match if they aren't for the same drive.
47+
if (!driveGlob.hasMatch(glob)) continue;
3848
// `C:\` => `\` for rejoining via posix.
3949
glob = glob.replaceFirst(_driveLetterMatcher, r'/');
4050
filteredGlobs.add(path.posix.joinAll(path.windows.split(glob)));

test/model_utils_test.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ void main() {
1818
test('basic Windows', () {
1919
expect(matchGlobs([r'C:\a\b\*'], r'c:\a\b\d', isWindows: true), isTrue);
2020
});
21+
22+
test('Windows does not pass for different drive letters', () {
23+
expect(matchGlobs([r'C:\a\b\*'], r'D:\a\b\d', isWindows: true), isFalse);
24+
});
2125
});
2226

2327
group('model_utils stripIndentFromSource', () {

0 commit comments

Comments
 (0)