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

Encode URI paths with slashes #1378

Merged
merged 2 commits into from
Feb 21, 2021
Merged

Conversation

wmertens
Copy link
Contributor

It's fine for query parameters to have visible / in them, and it makes it easier to work with the ungit URLs

Copy link
Contributor

@Hirse Hirse left a comment

Choose a reason for hiding this comment

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

I like the general idea, but see my comments for details.

source/utils/encode-path.js Outdated Show resolved Hide resolved
@@ -3,6 +3,7 @@ const octicons = require('octicons');
const components = require('ungit-components');
const navigation = require('ungit-navigation');
const programEvents = require('ungit-program-events');
const encodePath = require('../../source/utils/encode-path');
Copy link
Contributor

Choose a reason for hiding this comment

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

The components intentionally don't have any local imports as they are self-contained plugins.

To make encode-path available to components, export it as one of the existing modules or create a new one:

ungit/scripts/build.js

Lines 43 to 48 in 7b03413

b.require(path.join(publicSourceDir, 'components.js'), { expose: 'ungit-components' });
b.require(path.join(publicSourceDir, 'main.js'), { expose: 'ungit-main' });
b.require(path.join(publicSourceDir, 'navigation.js'), { expose: 'ungit-navigation' });
b.require(path.join(publicSourceDir, 'program-events.js'), { expose: 'ungit-program-events' });
b.require(path.join(publicSourceDir, 'storage.js'), { expose: 'ungit-storage' });
b.require(path.join(baseDir, 'source/address-parser.js'), { expose: 'ungit-address-parser' });

Adding the encodePath function as export to address parser might make sense.

@wmertens
Copy link
Contributor Author

@Hirse changes applied :)

Copy link
Collaborator

@campersau campersau left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

@campersau campersau merged commit 3a8da4f into FredrikNoren:master Feb 21, 2021
@wmertens wmertens deleted the encodeURI branch February 21, 2021 18:55
@campersau campersau mentioned this pull request Apr 2, 2021
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.

3 participants