Skip to content
This repository has been archived by the owner on Nov 5, 2020. It is now read-only.

Use library to cleanup story-source indentation #79

Closed
adrianjost opened this issue Mar 7, 2019 · 6 comments
Closed

Use library to cleanup story-source indentation #79

adrianjost opened this issue Mar 7, 2019 · 6 comments
Labels

Comments

@adrianjost
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When I create a bigger story with a template that is more than one line long, the "story-source" output is usually much too indented (the way it is defined in the template).

Example:

	.add("test", () => ({
		template: `
			<div>
				div is starting 3 tabs indented
			</div>`,

Output: (way to much indented)

// Source:
			<div>
				div is starting 3 tabs indented
			</div>`,

Describe the solution you'd like
I found the library outdent which remove the leading indentation to start without indentation. This makes the output way cleaner. But using this for all templates seems a bit messy so I thought, why not implementing such a generic problem directly inside the source-rendering function of this plugin.

As far as I understand, you just need to add outdent to this line of code: StorySource/index.vue#L20

import outdent from 'outdent';
// ...
computed: {
  sourceCode() {
    return this.lang === 'jsx' ? outdent`;${this.source}` : outdent`${this.source}`
  }
},
@pocka
Copy link
Owner

pocka commented Mar 8, 2019

@adrianjost
Thank you for pointing out!
The addon uses dedent to de-indent but after the little investigation for this issue, I found that it does not support tabs 😨
In contrast to it, outdent, which you suggested supports tabs. Though its behavior seems quite weird to me.

outdent(`
  foo
bar
  baz
`)

// "foo\nr\nbaz"
// "ba" is gone!

The idea came to my mind is "replacing all tabs with spaces(2 or 4 or 8, we can configure in options)". How about this?

Since I'm a 2space follower, I want to hear opinions from tab users ❤️

@pocka pocka added bug help wanted version: minor When you add functionality in a backwards-compatible manner labels Mar 8, 2019
@adrianjost
Copy link
Contributor Author

@pocka Thank you for your investigation. That's a shame for dedent ^^ So, as you pointed out that this plugin already uses a de-indent library, it seems to be a better idea to improve this library rather than try to fix it with a hack here.
In my opinion, replacing all tabs with spaces only visually solves the problem. For us Storybook also serves as a list of examples we can copy, so it would be strange to copy code with spaces to a codebase with tabs. (Even if Prettier should fix it immediately)

==> I try to make a contribution to dedent to enable the support of tabs and will keep you up to date here. Sounds good?


BTW: I was a 2 space follower for years but this article changed my mind (But I really don't wan't to start a discussion here, I'm fine with both)

@adrianjost
Copy link
Contributor Author

For your info: this is the pull request that should fix the issue: dmnd/dedent#28

@pocka
Copy link
Owner

pocka commented Mar 13, 2019

@adrianjost
Thanks for your immediate action! It's true that replacing with spaces is an inconvenience for who wants to copy & paste.
I'll close this issue because it's dedent's one. If they reject the PR or release it in a next major version, please reopen this (or mention me).

@pocka pocka closed this as completed Mar 13, 2019
@pocka pocka removed help wanted version: minor When you add functionality in a backwards-compatible manner labels Mar 13, 2019
@adrianjost
Copy link
Contributor Author

adrianjost commented Apr 19, 2019

Hi @pocka, it's been over a month now, since the dedent pr was open and there was no action from the mantainer at all. The last commit in the repo was also over a year ago.

What do you think about changing the package to my fork that is available at npm as well (dedent-tabs). I personally don't think that dedent will get an update this year or even in the next.

@pocka
Copy link
Owner

pocka commented Apr 20, 2019

@adrianjost

What do you think about changing the package to my fork that is available at npm as well (dedent-tabs). I personally don't think that dedent will get an update this year or even in the next.

I think it's good changing the dependency to your one :shipit: .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants