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

[BUG] appendToPath should add appropriate separators if missing #82

Open
woelmer opened this issue May 7, 2020 · 3 comments
Open

[BUG] appendToPath should add appropriate separators if missing #82

woelmer opened this issue May 7, 2020 · 3 comments

Comments

@woelmer
Copy link

woelmer commented May 7, 2020

  • Issue Type: BUG
  • Dart SDK version(s): 2.8.1
  • fluri Version(s): 1.2.8

With the following example where the base path does not have a trailing path separator, I would expect appendToPath to add the missing separator between the base and the path being appended.

Fluri base = new Fluri('https://example.com/base');

Fluri fluri = new Fluri.from(base)
  ..appendToPath('path/to/resource')

This results in a path of 'https://example.com/basepath/to/resource'


FYI: @dustinlessard-wf @evanweible-wf @jayudey-wf @maxwellpeterson-wf @sebastianmalysa-wf @trentgrover-wf

@woelmer woelmer changed the title appendToPath should add appropriate separators if missing [BUG] appendToPath should add appropriate separators if missing May 7, 2020
@evanweible-wf
Copy link
Contributor

I honestly can't remember if it was implemented like this intentionally or not, but at this point I'm a bit nervous to make this change without a major release in case there are existing usages that rely on this behavior. I do think adding the separator is probably the better option, though, so I'll keep this open and maybe we can find a reason to release a major. I've been meaning to take a look at extension methods now that they're available to see if some of this package's functionality could be an extension on Uri directly, so that would be another reason to release a major.

@woelmer
Copy link
Author

woelmer commented May 8, 2020

This is a pretty critical bug when your base url comes from a server and you have no idea whether there is a path separator on the end or not (obviously one could check but that defeats the purpose of this library entirely).

@evanweible-wf
Copy link
Contributor

Totally understand that it's not ideal. But this behavior has existed for over 4 years, so changing this behavior in a patch release would risk breaking existing usages, which in my opinion is worse than this not working as expected for new usages.

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

No branches or pull requests

2 participants