Memnarch / Delphinus

An alternative Packagemanager for the Delphi-IDE
Mozilla Public License 2.0
235 stars 64 forks source link

Allow directory normalisation of slashes. #80

Closed darnocian closed 3 years ago

darnocian commented 3 years ago

Users may want to use forward slashes in config where Windows uses back slashes.

Candidates are picture, license and paths. It may not be obvious that this can cause a problem that is not obvious without clearer error messages identifying the problematic element.

Memnarch commented 3 years ago

Hi, I'm curious, do you have a usecase for this?

darnocian commented 3 years ago

Hi

People are often writing code in multiple environments (windows/unix). It may be that sometimes, a / rather than \ is used.

I was trying to release my first package last week, and I spent considerable time trying to figure out why the package would not install. Once I debugged the plugin, I found that it was that the slash was wrong.

So there are two options: 1) be forgiving, and change the slash 2) detect the slash and improve the error messages everywhere so people know what the errors actually are an not have to debug... ;)

Memnarch commented 3 years ago

Sounds Reasonable. I assume it simply failed in a weird way, somewhere?

darnocian commented 3 years ago

Regarding the failures... I can't remember the exact message, but the output was something like 'Copying sources'... and then 'failure to find path' or something like that... But there was no clue as to where the issue was without debugging. ;( I was spending time editing the Install.json where my problem was actually in the Info.json.

The other reason I sometimes prefer using a forward slash (/) is that back slash () is often used for escaping things. So when looking at the json, without knowing the parser behaviour, you may wonder, do I double backslash (\) ... probably, which doesn't always look nice in my opinion.

I can't recall how exceptions bubbled up the call stack, but an alternative could be to change the the function to something like: function GetValidatedRelativePath(const ARelPath, AContext : string) : string; In this scenario, if we chose to enforce \, we can raise an exception using the AContext as a hint to the problem area.

Memnarch commented 3 years ago

The other reason I sometimes prefer using a forward slash (/) is that back slash () is often used for escaping things.

That is specified by the Json standard :) A parser which does not use \ for escaping in Json is wrong.

darnocian commented 3 years ago

Yup. More often than not people generate config files rather than computers... which is why I generally try to be friendlier to them. ;)

Memnarch commented 3 years ago

Thanks for the changes :) EDIT: And sorry for it taking so long

darnocian commented 3 years ago

No problem. A pleasure. Thanks for accepting them.

Memnarch commented 3 years ago

Always happy to get some help here and there^^