Skip to content

Conversation

@prabhjyotsingh
Copy link
Contributor

What is this PR for?

Every time user clicks on "Run all paragraphs" button system keeps appending an empty paragraph. Ideally content of paragraph should be checked before adding any empty paragraph.
This started happening after ZEPPELIN-707 was merged.

What type of PR is it?

[Bug Fix | Improvement]

Todos

  • - add more condition before calling note.addParagraph()

What is the Jira issue?

Screenshots (if appropriate)

Before
before

After
after

Questions:

  • Does the licenses files need update? n/a
  • Is there breaking changes for older versions? n/a
  • Does this needs documentation? n/a

@r-kamath
Copy link
Member

r-kamath commented Jul 1, 2016

LGTM

@corneadoug
Copy link
Contributor

@jongyoul Since you were reviewing the original PR can you check this one out?

@prabhjyotsingh
Copy link
Contributor Author

Merging this if no more discussion.

@jongyoul
Copy link
Member

jongyoul commented Jul 2, 2016

I'm testing it. Just wait.

@jongyoul
Copy link
Member

jongyoul commented Jul 2, 2016

@prabhjyotsingh After run all paragraphs, note adds new paragraph at the end of last paragraph. And then, if the last paragraph is empty, note don't create paragraph anymore. Is it normal? I think it's better not to create empty paragraph. Because running paragraph means running only, not creating anything. How about you?

@prabhjyotsingh
Copy link
Contributor Author

Yes, its working for me, if you refer to the gif attached above; if paragraph content is only interpreter name (like in this example "%md ") it wouldn't add a paragraph below, but if we append anything after it, on running that should append a paragraph below.

IMO, I too am in a favour of not adding an empty paragraph on running notebook.

@jongyoul
Copy link
Member

jongyoul commented Jul 2, 2016

@prabhjyotsingh I only said that case of "running all paragraph". I think "running all paragraphs" is not interactive mode. But in a normal case, I like to add empty paragraph at the end of paragraph. What I told is that how about not adding empty paragraph when you click "running all paragraphs".

@prabhjyotsingh
Copy link
Contributor Author

@jongyoul Sure, now I understand what you mean, but before that what do we call an empty paragraph; should it be Strings.isNullOrEmpty(text) or "%getInterpreterName()".equals(text.trim()) or both.
And if we say both then this is kind of taking care of it. However I can try and tune it.

@jongyoul
Copy link
Member

jongyoul commented Jul 2, 2016

@prabhjyotsingh You're right for the before behaviour. I thank you for taking care of it. In case of your PR, I've tested and worked well. I hope you can deal with my suggestion.

@prabhjyotsingh
Copy link
Contributor Author

Sure, thank you @jongyoul I'll open up a separate PR for fixing that.

@asfgit asfgit closed this in a7a6539 Jul 3, 2016
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
### What is this PR for?
Every time user clicks on "Run all paragraphs" button system keeps appending an empty paragraph. Ideally content of paragraph should be checked before adding any empty paragraph.
This started happening after [ZEPPELIN-707](https://issues.apache.org/jira/browse/ZEPPELIN-707) was merged.

### What type of PR is it?
[Bug Fix | Improvement]

### Todos
* [x] - add more condition before calling note.addParagraph()

### What is the Jira issue?
* [ZEPPELIN-1094](https://issues.apache.org/jira/browse/ZEPPELIN-1094)

### Screenshots (if appropriate)

Before
![before](https://cloud.githubusercontent.com/assets/674497/16512206/5d9b91b4-3f76-11e6-991f-560817efb331.gif)

After
![after](https://cloud.githubusercontent.com/assets/674497/16512205/5d993bbc-3f76-11e6-916a-2924a17bd6a1.gif)

### Questions:
* Does the licenses files need update? n/a
* Is there breaking changes for older versions? n/a
* Does this needs documentation? n/a

Author: Prabhjyot Singh <[email protected]>

Closes apache#1111 from prabhjyotsingh/ZEPPELIN-1094 and squashes the following commits:

7919648 [Prabhjyot Singh] add more condition before calling note.addParagraph()
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Dec 9, 2017
### What is this PR for?
Every time user clicks on "Run all paragraphs" button system keeps appending an empty paragraph. Ideally content of paragraph should be checked before adding any empty paragraph.
This started happening after [ZEPPELIN-707](https://issues.apache.org/jira/browse/ZEPPELIN-707) was merged.

### What type of PR is it?
[Bug Fix | Improvement]

### Todos
* [x] - add more condition before calling note.addParagraph()

### What is the Jira issue?
* [ZEPPELIN-1094](https://issues.apache.org/jira/browse/ZEPPELIN-1094)

### Screenshots (if appropriate)

Before
![before](https://cloud.githubusercontent.com/assets/674497/16512206/5d9b91b4-3f76-11e6-991f-560817efb331.gif)

After
![after](https://cloud.githubusercontent.com/assets/674497/16512205/5d993bbc-3f76-11e6-916a-2924a17bd6a1.gif)

### Questions:
* Does the licenses files need update? n/a
* Is there breaking changes for older versions? n/a
* Does this needs documentation? n/a

Author: Prabhjyot Singh <[email protected]>

Closes apache#1111 from prabhjyotsingh/ZEPPELIN-1094 and squashes the following commits:

7919648 [Prabhjyot Singh] add more condition before calling note.addParagraph()
@prabhjyotsingh prabhjyotsingh deleted the ZEPPELIN-1094 branch February 25, 2018 03:45
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.

4 participants