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

Clear URL when clearing terminal #170

Merged
merged 3 commits into from Sep 4, 2018
Merged

Clear URL when clearing terminal #170

merged 3 commits into from Sep 4, 2018

Conversation

n4bb12
Copy link
Contributor

@n4bb12 n4bb12 commented Sep 3, 2018

Follow-up of #169

@n4bb12 n4bb12 changed the title Clear URL when clearing terminal WIP ----- Clear URL when clearing terminal Sep 3, 2018
@n4bb12 n4bb12 changed the title WIP ----- Clear URL when clearing terminal Clear URL when clearing terminal Sep 3, 2018
@n4bb12
Copy link
Contributor Author

n4bb12 commented Sep 3, 2018

The terminal also gets cleared when hitting Ctrl + L. https://terminal.jcubic.pl/api_reference.php#shortcuts

If I can find out how to catch that, then I'll add that, too.

web/index.html Outdated
updateUrl("/");
term.clear();
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sharkdp sharkdp merged commit 790b946 into sharkdp:master Sep 4, 2018
@n4bb12
Copy link
Contributor Author

n4bb12 commented Sep 4, 2018

@sharkdp The issue with removing the leading / is that it breaks clear. The URL can only be cleared by replacing the history state with /. But that triggers the error here #171.

The problem does not occur if you serve the folder with an actual web server instead of just opening the file.

I think you need to decide here if a working clear is more important, or being able to open the web page without a web server.

@sharkdp
Copy link
Owner

sharkdp commented Sep 4, 2018

Oops. I was too quick with merging, sorry 😄

I think you need to decide here if a working clear is more important, or being able to open the web page without a web server.

If there is no way to extract the base url (/ or ../index.html), I think I would choose the latter option (be able to just open the index.html in a browser).

@n4bb12
Copy link
Contributor Author

n4bb12 commented Sep 4, 2018

Since you're already using npm, I'd suggest to add https://www.npmjs.com/package/live-server to the project. Devs can run npm start and get a working server without the need to install anything on top. As a nice bonus you also get automatic refresh when editing the html.

@n4bb12
Copy link
Contributor Author

n4bb12 commented Sep 4, 2018

I tested a few things and this worked for me, too:

// This resets the URL when opening the app as a file
history.replaceState(null, null, "index.html")

We can distinguish a local file from a web server by checking

location.href

If you want to go with that instead

@n4bb12 n4bb12 deleted the accept-input-from-url branch September 4, 2018 19:22
@sharkdp
Copy link
Owner

sharkdp commented Sep 4, 2018

I think we could store location.href once in the beginning:

var origin = location.href;

and then simply revert to that when clear is called. What do you think?

@n4bb12
Copy link
Contributor Author

n4bb12 commented Sep 4, 2018

Yea that.

You can't directly revert to it since the history API only accepts relative paths, but it does contain the necessary info to make it work.

Want a PR? 😄

@sharkdp
Copy link
Owner

sharkdp commented Sep 4, 2018

Hm, it seems to be okay if the base URL is still the same. But that might be just Chrome.

@n4bb12
Copy link
Contributor Author

n4bb12 commented Sep 4, 2018

You are right. What it actually doesn't accept is a URL with a different domain.

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.

None yet

2 participants