Separate server name and id, use hostname-valid encoding by duncanbeevers · Pull Request #216 · typicode/hotel

duncanbeevers · 2017-08-07T06:05:28Z

If you name your ~/.hotel/servers files using non-uri characters, they aren't recognized by the exists middleware.

This PR allows intuitive, human-friendly naming of these files, and a more beautiful list of servers.

duncanbeevers · 2017-08-07T08:38:22Z

src/daemon/group.js

-    const id = req.params.id
-      ? this.resolve(req.params.id)
-      : this.resolve(req.hostname.replace(tld, ''))
+    const tld = new RegExp(`\\.${daemonConf.tld}$`)


The previous regexp here wasn't enforcing a dot.

duncanbeevers · 2017-08-07T08:39:03Z

src/daemon/normalize.js

+module.exports = function normalize(id) {
+  return id
+    ? punycode
+        .encode(id)


punycode won't mess with existing ASCII; it only ensures that non-ASCII characters get embedded into a hostname-valid space.

duncanbeevers · 2017-08-07T08:40:22Z

src/daemon/normalize.js

+    ? punycode
+        .encode(id)
+        .toLowerCase()
+        .replace(/[^a-z0-9-.]/g, '-')


This ensures that ASCII characters such as spaces, parentheses, commas, and so on, are encoded to a hostname-valid space.

duncanbeevers · 2017-08-07T08:41:21Z

src/daemon/normalize.js

+        .encode(id)
+        .toLowerCase()
+        .replace(/[^a-z0-9-.]/g, '-')
+        .replace(/-+/g, '-')


Because so many characters are replaced by hyphen, any run of multiple hyphens is collapsed to a single hyphen.

duncanbeevers · 2017-09-09T05:09:05Z

@typicode ping

duncanbeevers · 2018-01-11T16:11:23Z

This PR has been updated to incorporate the changes from the UI refactor.

duncanbeevers · 2018-01-11T17:13:41Z

src/app/components/Nav/index.css

 }
+
+.listLink {
+  line-height: 1em;


This change prevents monitor names from collapsing.

Before

After

typicode · 2018-01-12T01:41:49Z

src/daemon/normalize.js

@@ -0,0 +1,12 @@
+const punycode = require('punycode') // eslint-disable-line node/no-deprecated-api
+
+module.exports = function normalize(name) {


Why the ternary, have you spotted a case where it can can be undefined or is it for safety?
Could you write some tests for normalize or add a comment with examples of inputs and expected outputs?

typicode · 2018-01-12T01:44:40Z

src/daemon/normalize.js

+        .toLowerCase()
+        .replace(/[^a-z0-9-.]/g, '-')
+        .replace(/-+/g, '-')
+        .replace(/-$/, '')


Not sure to understand what they're for?

.replace(/-+/g, '-') .replace(/-$/, '')

It’s to ensure that multiple invalid characters only get replaced by one -, and to remove trailing -s.

Yes, this is just for tidiness sake. In the examples I posted server names include whitespace followed by non-alpha-numeric characters resulting in double hyphens.

It looks a little cleaner to me to tidy them, but the code I wrote mostly applies to my own naming scheme. For example, it doesn't remove leading hyphens, but that's only because I'd never named a server something that would result in a leading hyphen.

I'm happy to simplify this entire section and simply use the pure punycode replacements. That seems less likely to result in id collisions where multiple similarly-named servers normalize to the same server id.

typicode · 2018-01-12T01:46:32Z

package.json

    "npm-run-all": "^4.1.2",
    "pkg-ok": "^1.0.1",
    "prettier": "^1.10.2",
+    "punycode": "^2.1.0",


Could you move it to dependencies as it's used in the daemon?

typicode · 2018-01-12T01:51:44Z

Thanks for the PR :) (and the style fix)

I'd like to understand better, the idea is to be to have "pretty" names for files and automatically get valid domain?

Something like:

~/.hotel/servers/My Server (something).json # my-server-something.localhost

What does your projects directories look like? Do they have () as well or is it just something you want to have in hotel UI?

duncanbeevers · 2018-01-12T20:12:11Z

@typicode The filenames represent the displayed values, like so:

❯ ls -1 ~/.hotel/servers/
Always Master (staging).json
Always Master.json
Auth (staging).json
Auth.json
PMP (desktop).json
PMP (tv).json
Style Guide.json
desktop (staging).json
desktop.json
hotel.json
reflector.json
tv.json

In some cases, parameterizable hotel servers would alleviate this, as many of the configurations are very similar to one another with the modification of one or two command-line flags.

…hostname

duncanbeevers force-pushed the decode-uri-component-hostname branch 2 times, most recently from c2ea9cd to a6a7c22 Compare August 7, 2017 08:29

duncanbeevers changed the title ~~Decode URI entities in id param and hostname~~ Separate server name and id, id uses hostname-valid encoding Aug 7, 2017

duncanbeevers changed the title ~~Separate server name and id, id uses hostname-valid encoding~~ Separate server name and id, use hostname-valid encoding Aug 7, 2017

duncanbeevers force-pushed the decode-uri-component-hostname branch from a6a7c22 to cb2814f Compare August 7, 2017 08:35

duncanbeevers commented Aug 7, 2017

View reviewed changes

duncanbeevers force-pushed the decode-uri-component-hostname branch from cb2814f to 0d45259 Compare August 7, 2017 15:29

duncanbeevers force-pushed the decode-uri-component-hostname branch from 0d45259 to 1a40140 Compare December 26, 2017 18:57

duncanbeevers force-pushed the decode-uri-component-hostname branch from 1a40140 to 04f3e65 Compare January 11, 2018 16:10

duncanbeevers commented Jan 11, 2018

View reviewed changes

typicode reviewed Jan 12, 2018

View reviewed changes

duncanbeevers added 3 commits January 15, 2018 20:15

Separate hostname and displayed name of servers; use punycode hostname

0a6df84

fixup! Separate hostname and displayed name of servers; use punycode …

1a00ec5

…hostname

fixup! Separate hostname and displayed name of servers; use punycode …

8813baf

…hostname

duncanbeevers force-pushed the decode-uri-component-hostname branch from 34e21c3 to 8813baf Compare January 16, 2018 04:18

		@@ -0,0 +1,12 @@
		const punycode = require('punycode') // eslint-disable-line node/no-deprecated-api

		module.exports = function normalize(name) {

Uh oh!

Conversation

duncanbeevers commented Aug 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duncanbeevers commented Sep 9, 2017

Uh oh!

duncanbeevers commented Jan 11, 2018

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

typicode commented Jan 12, 2018

Uh oh!

duncanbeevers commented Jan 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

duncanbeevers commented Aug 7, 2017 •

edited

Loading