Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle more flexible patterns #134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ttutisani
Copy link
Owner

No description provided.

Copy link
Owner Author

@ttutisani ttutisani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miguelius I reviewed and I have some questions. Please respond and I will think more about this.

List<string> GetFeatureFilePaths();
FeatureFile GetByFilePath(string filePath);
List<string> GetFeatureFilePaths();
List<string> FindFilesByPattern(string pattern);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not belong here conceptually. This is a feature file repository not a file path repository. What are you trying to do with it?

@@ -34,9 +34,10 @@ public List<FeatureFile> Discover(Type featureClassType)

if (fileClassInfo.IsPattern) {
_featureFileRepository
.GetFeatureFilePaths()
.FindAll(f => fileClassInfo.MatchesFilePathPattern(f))
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this what you are trying to fix? MatchesFilePathPattern - this returns false but must return true. Am I missing something?

@ttutisani
Copy link
Owner Author

What happens if the path contains multiple asterisk symbols such as in the folder names? e.g., ./Features*Test/*.feature. Will this work fine? I think this will have a problem, correct? I am asking before this PR is merged, will it work? This PR will not be able to address such use case for sure.

@miguelius
Copy link
Contributor

miguelius commented Sep 29, 2022 via email

@miguelius
Copy link
Contributor

Tried it with dotnet-script and I get:

> Directory.EnumerateFiles(".","Desc*/*.doc")
System.IO.DirectoryNotFoundException: System.IO.DirectoryNotFoundException: Could not find a part of the path '/home/u86415/Desc*'.
  + FileSystemEnumerator<TResult>.CreateDirectoryHandle(string, bool)
  + FileSystemEnumerator<TResult>.Init()
  + FileSystemEnumerator<TResult>..ctor(string, bool, System.IO.EnumerationOptions)
  + FileSystemEnumerable<TResult>..ctor(string, System.IO.Enumeration.FileSystemEnumerable<TResult>.FindTransform, System.IO.EnumerationOptions, bool)
  + System.IO.Enumeration.FileSystemEnumerableFactory.UserFiles(string, string, System.IO.EnumerationOptions)
  + System.IO.Directory.InternalEnumeratePaths(string, string, System.IO.SearchTarget, System.IO.EnumerationOptions)
  + System.IO.Directory.EnumerateFiles(string, string)
> Directory.GetFiles(".","Desc*/*.doc")
System.IO.DirectoryNotFoundException: System.IO.DirectoryNotFoundException: Could not find a part of the path '/home/u86415/Desc*'.
  + FileSystemEnumerator<TResult>.CreateDirectoryHandle(string, bool)
  + FileSystemEnumerator<TResult>.Init()
  + FileSystemEnumerator<TResult>..ctor(string, bool, System.IO.EnumerationOptions)
  + FileSystemEnumerable<TResult>..ctor(string, System.IO.Enumeration.FileSystemEnumerable<TResult>.FindTransform, System.IO.EnumerationOptions, bool)
  + System.IO.Enumeration.FileSystemEnumerableFactory.UserFiles(string, string, System.IO.EnumerationOptions)
  + System.IO.Directory.InternalEnumeratePaths(string, string, System.IO.SearchTarget, System.IO.EnumerationOptions)
  + System.IO.Directory.GetFiles(string, string, System.IO.EnumerationOptions)
  + System.IO.Directory.GetFiles(string, string)

It looks like it doesn't support wildcards but in the file name. Basically like dir command.

@ttutisani
Copy link
Owner Author

Okay, thanks for trying it! Is that based on my master branch or your fork?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants