-
Notifications
You must be signed in to change notification settings - Fork 257
Issue with multiple indirect relative paths #71
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
Comments
Txs. Will look at it |
It's an interesting one. It actually works as designed. How many times we've heard that? :) > ...has none of the subfolders (neither "\Library" nor "\Library\Group") in its extraDirs array... Correct. This is exactly what it should do. Thus if you did not specify explicitly any search dirs then all imports will be resolved with respect to the "root point", which is the current directory of the process. And this is exactly what is happening in your case. This also explains why Now, there is a solution to your problem. You can reset the "root point" of the import statement by using |
Yes, the suggested change to |
I think there is a slight misunderstanding here. The single-dot prefix is not a solution for your problem as I am not convinced there is a problem to solve. I only recommended to use it because it can help you to avoid specifying probing directories (search dirs) explicitly. Something that you wanted. However a proper reliable approach is in fact to specify search dirs explicitly. You proposal to allow search dirs being auto-added while resolving the files, despite being attractive, can lead to the unpredictable behavior. Simply because in this case the overall outcome of the assembly probing is non-deterministic. Thus unfortunately the proposal cannot be accepted. > DOS as well as UNIX fully understand ".." without an additional "." prefix. > Also note that I haven't found that hint anywhere in the help.
However, you are right the online help does not contain the "single-dot prefix" description. It will be fixed asap. Thank you, |
I fully understand that "allow search dirs being auto-added" can lead to undesired side effects, so I understand that this is a no-go. But: Why not allow the single-dot behavior for double-dot prefixes as well? I.e. "If a relative file path is specified with single-dot or double-dot prefix it will be automatically converted into the absolute path with respect to the location of the file containing the directive being resolved."? I think that would be very intuitive, since DOS/UNIX/... also accept "." as well as ".." when starting a relative path. |
I actually like your idea even though it mixes up two different concepts
Allowing single-dot behavior for double-dot prefixes unconditionally has a side effect as it would complicate referencing scripts from global highly structured folders. It will also potentially upset the users who already relying on "single vs double" being handled differently. But... I am not sure how many users indeed capitalize on the current flexibility of "single vs double". And I do agree that having the unified probing algorithm is more intuitive. Thus I have implemented your proposal in the latest codebase though as an option. The pre-release v3.26.1.0 does not have it but when it is to be finally released it will contain the change. In order to activate it you will need to have it activated from the config file: ...
<enableDbgPrint>True</enableDbgPrint>
<resolveRelativeFromParentScriptLocation>True</resolveRelativeFromParentScriptLocation>
</CSSConfig> Initially it will be set to |
Hi Oleg, I have upgraded to 3.27.0 and tried to verify issue #71. Unfortunately it doesn't work as expected, I am still getting a FileNotFoundException:
When prefixing the
I am attaching my test scripts. You can reproduce the issue as follows:
I have a second bunch of test scripts verifying In my own test environment I actually have both tests in the same folder structure, but for this issue description I have split them to simplify things a bit. Best regards, |
- Added `ResolveRelativeFromParentScriptLocation` for ALL non absolute paths in `//css_import`
Hi Matthias, The current release has this feature enabled only for imports staring with "../". if (ResolveRelativeFromParentScriptLocation && statement.Length > 1 && statement.StartsWith(".."))
statement = Path.Combine(Path.GetDirectoryName(parentScript), statement); I just wasn't sure about applying this approach to the "naked paths" (relative path that starts with a character) like in your samples. I have to say, it was rather an intuitive but not a logical decision. And after looking at your samples I am convinced that the approach needs to be extended the whole way. Thus 0d398ee commit delivers the support for "naked files" as well. You can test it without waiting for the release. The binaries can be found here: https://github.com/oleg-shilo/cs-script/tree/master/Source/Build. Just ensure you have enabled
There is one extra point I want to make. You have attached The reason for this is that this new handling "naked paths" in the algorithm makes the feature incompatible with referencing. No "naked assemblies" should be converted into absolute path as it would really screw the C# compiler. Remember, .NET compilers reserve "naked paths" for GAC assemblies. This is where that single dot makes perfect sense as it makes possible to distinguish between local ( This in tern makes me question my earlier objective of "having the unified probing algorithm". It is clear to me now that the "auto-doting" naked paths can only work reliable for importing but not referencing. Thus if even referencing ever gets "auto-doting" support, I think it is safer to keep Thank you, |
Upps, I thought I had earlier commented on your question, but somehow it got lost... So again:
And, I personally prefer keeping Best regards, |
Minor usability improvements * Added System.dll auto-referencing on EnableDbgPrint:true to allow regular expressions to be used in auto-injected dbg.cs. * Issue #78: Beautify command line help output * Issue #71: Issue with multiple indirect relative paths * Added ResolveRelativeFromParentScriptLocation for ALL non absolute paths in //css_import
Hmm, is there still something wrong are am I verifying wrongly? I still get the same error with the test scripts:
When keeping the ".\" prefix at the three following locations, all works fine again:
Something at first puzzled me:
But of course this is all expected behavior of the cache. Still, as I mentioned earlier, I'd prefer a default setting of So, was it intended to keep the default setting to |
For the case it was indeed intended to keep |
I am sorry, my wording in my previous post wasn't definitive enough:
What I meant to say was that ResolveRelativeFromParentScriptLocation is going to stay "disabled" by default. I know it goes against you personal preference but in this case I decided to make the "consistency" a primary goal even by slightly sacrificing the usability in the edge cases like yours. The decision is really based on a simple risk/benefit assessment. Particularly because the mechanism for a simple overwriting the default behavior is available. I am glad you have figured out the way around. But you can do it even in a simpler way. Just call it once and the setting will be set globally: CSScript.GlobalSettings.ResolveRelativeFromParentScriptLocation = true; |
I understand that the risks are too high for changing something that many users are likely to rely upon. Thanks for the hint with By the way, I have just noticed a minor inconsistency with So, this item #71 is verified and completed for me. Thanks! |
Fair point. Done. Will be delivered in the next release. |
Hi Oleg,
I though that all issues with relative paths have been solved, but I have just managed to trick the file resolver again when writing a script library.
Here's the situation:
Script.cs
containing//css_import .\Library\Group\ItemContainerOfGroup.cs;
LibraryBasics.cs
(not importing anything else)ItemThatDerivesFromLibraryBasics.cs
containing//css_import ..\LibraryBasics.cs;
ItemContainerOfGroup.cs
containing//css_import .\ItemThatDerivesFromLibraryBasics.cs;
I agree that this is a somewhat complicated structure, but the library will contain more than 100 items, so I decided to group them in subfolders, each containing a container that collects all items of a group.
Now, when CSScript compiles
Script.cs
, it properly resolvesItemContainerOfGroup.cs
as well asItemThatDerivesFromLibraryBasics.cs
, but then fails to resolve theLibraryBasics.cs
base class, even thoughItemThatDerivesFromLibraryBasics.cs
properly refers to that file with a relative path.The reason is that the file resolver (i.e.
fileparser.ResolveFiles()
) has none of the subfolders (neither "\Library" nor "\Library\Group") in itsextraDirs
array. This puzzles me, after all, CSScript has properly processed the//css_import .\Library\Group\ItemContainerOfGroup.cs;
statement. Shouldn't such additional path somewhere be added down the chain toResolveFiles()
?Adding
//css_searchdir ..
toItemThatDerivesFromLibraryBasics.cs
doesn't help.But funnily, I can work around to issue by adding
//css_searchdir .\Library\Group;
toScript.cs
, which will combine the//css_import ..\LibraryBasics.cs;
to the correct location, resulting in the following combination of statements:This is something we call "Doppel-Moppel" in German, i.e. something that is duplicated even though the duplication is not really necessary.
Best regards,
Matthias
The text was updated successfully, but these errors were encountered: