Skip to content

Add 'Open in editor' button for runnable code examples#1669

Merged
dlang-bot merged 8 commits intodlang:masterfrom
wilzbach:open-in-editor
Jun 28, 2017
Merged

Add 'Open in editor' button for runnable code examples#1669
dlang-bot merged 8 commits intodlang:masterfrom
wilzbach:open-in-editor

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Jun 5, 2017

As I have waited for DPaste to add this feature for more than a year, I decided that it can't be too difficult to add to the Dlang-Tour.
This depends on a PR at the dlang-tour: dlang-tour/core#524

@wilzbach wilzbach changed the title [WIP] Add 'Open in editor' button for runnable code examples Add 'Open in editor' button for runnable code examples Jun 10, 2017
@wilzbach
Copy link
Contributor Author

This depends on a PR at the dlang-tour: dlang-tour/core#524

I just took the liberty to merge the respective PR and thus this is ready to be shipped :)

CC @ZombineDev @CyberShadow @andralex @stonemaster @s-ludwig

(I also added a commit to make the resulting generated example code look a bit nice).

An example: http://dtest.dlang.io/artifact/website-1154d9b8e57a33be99e173919dc29f95e5752248-9b40d757592687bf250fd6ea2e871d00/web/phobos-prerelease/std_algorithm_searching.html#.minElement

@CyberShadow
Copy link
Member

The obvious question is what does this add in addition to the existing "Edit" button? Opening the code on a separate page with no other clutter?

Also, an observation: with this change, there is now an "Edit" button and "Open in editor" button, which is a bit superfluous. How about moving the latter to the button row that's displayed after you click "Edit"?

@wilzbach
Copy link
Contributor Author

The obvious question is what does this add in addition to the existing "Edit" button? Opening the code on a separate page with no other clutter?

This was one of the first feature request when this was first announced on the NG.
FWIW it's not only a separate & clean page:

  • it exports your current code (with modifications) & you can share this link with anyone
  • it adds the "secret" main and imports, so people can truly copy/paste this code to their editor (I did get a couple of bug reports complaining that the examples don't run offline and it was always that adding the imports was forgotten ...)
  • the dlang-tour has a couple of goodies, e.g.
    • error highlighting
    • line numbers
    • formatting with dfmt
  • it's a nice redundancy (dlang-tour is far less down than DPaste)

image

Also, an observation: with this change, there is now an "Edit" button and "Open in editor" button, which is a bit superfluous. How about moving the latter to the button row that's displayed after you click "Edit"?

Hmm, I get your point, but the idea was to help people who don't want to edit their code inline on dlang.org (and might not realize that there's an "edit on external site" button in the edit mode).
That being said, I don't feel strongly about this & would be fine either way here ;-)

@CyberShadow
Copy link
Member

it exports your current code (with modifications) & you can share this link with anyone

This feature needs work - for one, the effect of the "Export" button is not obvious, for two, the URLs generated are gargantuan and not suitable for actual sharing. It could either use what godbolt's compiler explorer does (integrate a third-party URL shortener) or save/generate short URLs itself.

the dlang-tour has a couple of goodies, e.g.

No reason these can't be features of the inline editor as well.

it's a nice redundancy (dlang-tour is far less down than DPaste)

A problem in the implementation should not serve as a reason for changing the UI!

Hmm, I get your point,

Not sure if I made it sufficiently clear.

If you were to swap the labels of the two buttons, they would both remain technically correct. "Edit inline" and "Edit on separate page" would be an improvement in that area but clumsy overall.

Generally, I am really frustrated how many similar services we have with overlapping functionality. What we need is a single service on a single domain which has compilation, execution, disassembly, and sharing. Currently, the DPaste frontend is an unholy bloated abomination with features nobody wants, such as user account registration and two CAPTCHAs. IMO, the frontend should just be rewritten from scratch - with the feature set we actually need, reimplementing it should not take much effort.

So, I'm hesitant about UI changes which further cement this fragmentation, and I would suggest to instead spend time to consolidate these services. Let me know if you need access to the DPaste accounts.

@wilzbach
Copy link
Contributor Author

This feature needs work - for one, the effect of the "Export" button is not obvious, for two, the URLs generated are gargantuan and not suitable for actual sharing. It could either use what godbolt's compiler explorer does (integrate a third-party URL shortener) or save/generate short URLs itself.

Fair point.

No reason these can't be features of the inline editor as well.

Maintaining dlang.org is already painful enough :S

it's a nice redundancy (dlang-tour is far less down than DPaste)
A problem in the implementation should not serve as a reason for changing the UI!

Hehe, btw that's also I reason why I experimented with replacing DPaste as backend with the DLang-Tour:
#1647

If you were to swap the labels of the two buttons, they would both remain technically correct. "Edit inline" and "Edit on separate page" would be an improvement in that area but clumsy overall.

Okay understood, but I am interested on feedback/ideas from other people before starting to change this.

Generally, I am really frustrated how many similar services we have with overlapping functionality. What we need is a single service on a single domain which has compilation, execution, disassembly, and sharing. Currently, the DPaste frontend is an unholy bloated abomination with features nobody wants, such as user account registration and two CAPTCHAs. IMO, the frontend should just be rewritten from scratch - with the feature set we actually need, reimplementing it should not take much effort.
So, I'm hesitant about UI changes which further cement this fragmentation, and I would suggest to instead spend time to consolidate these services. Let me know if you need access to the DPaste accounts.

Hmm, I am not really interested in maintaining yet another service, sorry. My point was that I already maintain the Tour, which has a D execution backend and which can be - with little effort - be used as an editor as well.

@CyberShadow
Copy link
Member

Maintaining dlang.org is already painful enough :S

If these features were factored into a properly encapsulated JavaScript library then it should not have been difficult to add them to any website.

Hmm, I am not really interested in maintaining yet another service, sorry.

Well, "good enough" is the enemy of better, and we have been entrenching ourselves deeper and deeper into technical debt on many fronts across our entire infrastructure. At some point we are going to need to start saying "no" to further improvements which increase complexity and inter-dependencies in exchange for small benefits.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Fix Bugzilla Description
17560 Enhancement: view and copy full code example for offline compile/play

@wilzbach
Copy link
Contributor Author

@CyberShadow I found 17560 on Bugzilla. As predicted newbies have a hard time realizing that they need to import the current module (and maybe std.stdio).
Solution 1: always add these import to raw HTML (like on this PR)
Solution 2: go with this PR
Solution 3: go with Solution 1 & 2

@CyberShadow
Copy link
Member

I don't feel comfortable merging hacks that compensate for the inadequacy of our previous hacks. The first step of solving technical debt is to begin saying 'no' to quick fixes that entrench us deeper into technical debt.

@wilzbach
Copy link
Contributor Author

I don't feel comfortable merging hacks that compensate for the inadequacy of our previous hacks. The first step of solving technical debt is to begin saying 'no' to quick fixes that entrench us deeper into technical debt.

That's not what I asked ;-)
What's your point on the linked issue and how would you solve it?

@CyberShadow
Copy link
Member

What's your point on the linked issue and how would you solve it?

#1669 (comment)

Fix Issue 17560 - Enhancement: view and copy full code example for offline compile/play
@wilzbach
Copy link
Contributor Author

What's your point on the linked issue and how would you solve it?
comment

The question was whether we should add import std.X to the generated examples...
To repeat:

  • the underlying issue is that newbies have a hard time to realize that the need to add the respective (see e.g. this post)
  • I proposed to add the "open in editor" feature because it's a simple solution to the problem of deciding on whether we always add the imports of the respective module (and std.stdio) on all documentation pages
  • I am not going to rewrite DPaste as I already maintain a replacement (DTour), has mores features and is written in D (imho time should be spent on adding DUB support to the Tour, s.t. a user can play with any module)

Btw I just saw the example on https://www.rust-lang.org:

Two note-worthy points:

  • they show the button "open in external" on the result page
  • the urlEncode the entire text as well (so implementing & maintain a pastebin shouldn't be necessary)

@wilzbach
Copy link
Contributor Author

This feature needs work - for one, the effect of the "Export" button is not obvious,

I reorder the buttons as you suggested. Your proposed naming was too long, so I abbreviateD:

image

@CyberShadow
Copy link
Member

I proposed to add the "open in editor" feature because it's a simple solution to the problem of deciding on whether we always add the imports of the respective module (and std.stdio) on all documentation pages

Sounds fine by itself (not accounting for other blockers)

I am not going to rewrite DPaste as I already maintain a replacement (DTour), has mores features and is written in D (imho time should be spent on adding DUB support to the Tour, s.t. a user can play with any module)

Should I interpret that as you openly stating on record that you have a conflict of interest that is preventing you from contributing to dlang.org in an unbiased manner? :)

TBH I don't care what language or backend DPaste uses - it may very well be a 100-line vibe.d or PHP app with a simplistic database that uses the Tour backend, or a Tour page with some CSS on it to make it generic and non-specific to the tour. Still doesn't change that the fragmentation and technical debt is hurting us.

they show the button "open in external" on the result page

Nice. Can we use fa-external-link here?

the urlEncode the entire text as well (so implementing & maintain a pastebin shouldn't be necessary)

Um. They have a "shorten" button. Which was the first option I proposed.

@wilzbach
Copy link
Contributor Author

Nice. Can we use fa-external-link here?

Sure. We could even use FontAwesome icons for all buttons, e.g:

image

Should I interpret that as you openly stating on record that you have a conflict of interest

Hehe my points was more IIRC the D Language Foundation is already paying for the server of tour.dlang.org (I don't know details, but the instance has 4GB).

that is preventing you from contributing to dlang.org in an unbiased manner? :)

It's about dpaste.dzfl.pl - not dlang.org ;-)

Um. They have a "shorten" button. Which was the first option I proposed.

I thought you didn't like the long redirect URLs?
If it's just a "short" button, this isn't hard to do:

dlang-tour/core#544

TBH I don't care what language or backend DPaste uses - it may very well be a 100-line vibe.d or PHP app

Adding new features to a vibe.d app is fun, it isn't for PHP ;-)

@CyberShadow
Copy link
Member

Hehe my points was more IIRC the D Language Foundation is already paying for the server of tour.dlang.org (I don't know details, but the instance has 4GB).

#1647 (comment)

dlang-tour/core#544

Looks great!

Adding new features to a vibe.d app is fun, it isn't for PHP ;-)

We need to remove features, not add them. As I said, all that's needed is running and saving D code.

@CyberShadow
Copy link
Member

Sure. We could even use FontAwesome icons for all buttons, e.g:

Heh, I meant instead of the "Edit (external)" label. That would avoid stuttering. Maybe with "float: right". What do you think?

If you'd like to keep the icons on the other buttons, what do you think of putting them in front of the text? That seems more natural/common to me.

@wilzbach
Copy link
Contributor Author

Heh, I meant instead of the "Edit (external)" label. That would avoid stuttering. Maybe with "float: right".

Roughly like this?

image

What do you think?

I am not a designer, but I think we either go without buttons or with buttons for all actions - in-between looks a bit weird.

If you'd like to keep the icons on the other buttons

It was just an experiment. No hard feelings.

what do you think of putting them in front of the text? That seems more natural/common to me.

image

(this PR now contains this)

@CyberShadow
Copy link
Member

CyberShadow commented Jun 28, 2017

Roughly like this?

Err, more like this:

(but maybe with icons on the other buttons too)

@wilzbach
Copy link
Contributor Author

(but maybe with icons on the other buttons too)

This would look like this:

image

There's also the possibility of keeping the previous style:

image

Or fully "barebone":

image

@CyberShadow
Copy link
Member

CyberShadow commented Jun 28, 2017

Looks pretty good!

  1. There is more spacing between the buttons and example than between the symbol documentation and the buttons.
  2. Can the button on the right be aligned so it's flush with the right side of the example block?
  3. Title text (hover tooltip) on the last button (or all buttons)?

@CyberShadow
Copy link
Member

Awesome.

What do you think of putting the button bar below the examples (content first)?

@CyberShadow
Copy link
Member

Thanks! I'm not sure if you find all of my suggestions agreeable or if I'm being overly imposing with them :)

@wilzbach wilzbach force-pushed the open-in-editor branch 2 times, most recently from 9d06915 to 40af0a6 Compare June 28, 2017 13:18
@wilzbach
Copy link
Contributor Author

Awesome.

Sorry entirely forgot to post screenshots - forgot to press on "Comment"...

image

sel

What do you think of putting the button bar below the examples (content first)?

That's how we do it on the front page as well.

image

I forgot about the "Edit" page, hence the editing of the last commit, but I think we are all set now:

image

@wilzbach
Copy link
Contributor Author

Thanks! I'm not sure if you find all of my suggestions agreeable or if I'm being overly imposing with them :)

Hehe, I think it greatly helps to create a better end result and that's what matters ;-)

@dlang-bot dlang-bot merged commit 6ff5545 into dlang:master Jun 28, 2017
@andralex
Copy link
Member

Awesome possum. Thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants