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

Add short link generation #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jayhoogle
Copy link

Moved across changes from the main repo PR @viebel.

@viebel
Copy link
Owner

viebel commented Jan 16, 2021

reviewing it!

src/klipse/utils.cljs Outdated Show resolved Hide resolved
:headers {"accept" "application/json" "content-type" "application/json"}
:query-params {:format "simple" :url "https://google.com"}}))]
(if-not (= (:status response) 200)
(
Copy link
Owner

@viebel viebel Jan 16, 2021

Choose a reason for hiding this comment

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

better to move alert and print into a separate function outside of fetch-shortened-url

Copy link
Author

Choose a reason for hiding this comment

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

Done, although I can't seem to get the URL to output on to the Klipse output. Is there another function I need to call besides print to make that happen?

@viebel
Copy link
Owner

viebel commented Jan 16, 2021

@jayhoogle I am not able to generate a short link when testing locally.
Here is the scenario I am trying.

  1. Open http://localhost:5016/index-dbg.html
  2. Type (map inc [1 2 3])
  3. Press Ctrl-S

I am getting a bad request error
image

@jayhoogle
Copy link
Author

@viebel that's because localhost urls aren't accepted by the link shortening service. I can alter so that it passes a url with app.klipse.tech automatically prepended if that would be preferable?

@viebel
Copy link
Owner

viebel commented Jan 16, 2021

What we should do is to provide a way to disable the shortening url feature via a url parameter named no_short_url. Please us this function to read the url parameters

(def url-parameters (memoize url-parameters*))

@@ -9,6 +9,31 @@
[cljs.core.async :refer [timeout <! chan put!]]
[cemerick.url :refer [url]]))

(defn show-url
[url]
(
Copy link
Owner

Choose a reason for hiding this comment

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

Remove unnecessary wrapping parentheses

@viebel
Copy link
Owner

viebel commented Jan 16, 2021

@jayhoogle What are the license terms of is.gd ? I am not comfortable with the CORS hack that you used.

{:with-credentials? false
:headers {"accept" "application/json" "content-type" "application/json"}
:query-params {:format "simple" :url current}}))]
(if-not (= (:status response) 200)
Copy link
Owner

Choose a reason for hiding this comment

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

destructure status and body

@@ -29,9 +54,18 @@
(assoc-in [:query (name key)] value)
str))

(defn process-url [base-url input]
(let [a (add-url-parameter base-url :cljs_in input)]
Copy link
Owner

Choose a reason for hiding this comment

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

use a thread macro ->

(add-url-parameter :cljs_in input)))
(->
(if base-url (url base-url) (current-url))
(process-url input)
Copy link
Owner

Choose a reason for hiding this comment

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

always close parentheses on the above line

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