Skip to content
This repository was archived by the owner on Jun 8, 2022. It is now read-only.

Allow for other CE instance + create URL#4

Merged
emmatyping merged 6 commits intoemmatyping:masterfrom
dkm:master
Jun 13, 2018
Merged

Allow for other CE instance + create URL#4
emmatyping merged 6 commits intoemmatyping:masterfrom
dkm:master

Conversation

@dkm
Copy link
Contributor

@dkm dkm commented Jun 13, 2018

This should fix #1 and #3 .

I've also took the liberty of using a different mecanism for building JSON that is sent over the POST. It had the advantage to be easily copy/pasted across the python and shell script I'm already using :D

Copy link
Owner

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

This looks really nice. Thank you!

.post(format!("https://www.godbolt.org/api/compiler/{}/compile", &compiler).as_str())
.json(&source)
pub fn compile(client: Client, host: &str, src: String, compiler: &str, args: String) -> String {
let filters = json!(
Copy link
Owner

Choose a reason for hiding this comment

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

Wow, this is a lot nicer. Thanks!

src/url.rs Outdated
},
"options": args,
"compiler": compiler,
// "fontScale": 3.0
Copy link
Owner

Choose a reason for hiding this comment

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

Speaking of easy copy/pasting, I think you left this in and didn't mean to :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's true. There is so much that could be fine tuned :) I can rebase and remove this line...

@dkm dkm force-pushed the master branch 2 times, most recently from cb51acc to a602080 Compare June 13, 2018 18:47
@dkm
Copy link
Contributor Author

dkm commented Jun 13, 2018

rebased and pushed !

@emmatyping
Copy link
Owner

Excellent, thank you! I will merge once CI go green.

dkm added 5 commits June 13, 2018 20:50
Possible incorrect result from API #955

Returned JSON may have invalid objects. Mark the field as optional as the incorrect cases seems to appear on empty lines.
Use --host for changing the default compiler explorer.
Easier to share json with other tools if json is directly inline.
Basic support for creating an URL that points to a single code editor
and using a single compiler, both provided on the command line.
Add text for --url for compile command, and refresh --help listing.
@dkm
Copy link
Contributor Author

dkm commented Jun 13, 2018

I've added a single commit for updating the README (very basic).

@emmatyping emmatyping merged commit d7dbcb7 into emmatyping:master Jun 13, 2018
@emmatyping
Copy link
Owner

Thank you so much!

@emmatyping emmatyping mentioned this pull request Jun 13, 2018
@dkm
Copy link
Contributor Author

dkm commented Jun 13, 2018

Thanks for merging :)

For my information, how did you merge it ? Usually, merging PR maintains original commits (and authorship -- in this case, github changed the mail), but it seems that in this case all my commits were squashed before merging ? It's not really a problem in this case as it's rather small, but more by curiosity.

@emmatyping
Copy link
Owner

There is an option to squash and merge on Github if you hit the arrow next to the merge button. It's the default several other projects I work on use, so its somewhat become habit for me :)

FWIW authorship stays the same, it just mentions I committed.
image

@dkm
Copy link
Contributor Author

dkm commented Jun 14, 2018 via email

@emmatyping
Copy link
Owner

Ah, well if you want I wouldn't be opposed to adding an ACKS file for acknowledgements.

@dkm
Copy link
Contributor Author

dkm commented Jun 14, 2018 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow different remote url other than godbolt.org

2 participants