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

Issue #377 collapse button staying red #382

Closed
wants to merge 3 commits into from

Conversation

Eugene-msc
Copy link

Basically it checks if the current command is not empty. If it's not then the command was actually executed, otherwise last_command is discarded

@p-e-w p-e-w mentioned this pull request Nov 9, 2014
last_command = stream_element.get_text_parameter(0, "");
else
last_command = null;

Copy link
Owner

Choose a reason for hiding this comment

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

I like this solution a lot! There are two changes I would like to see before I merge:

  1. Please adapt the code style to what is used elsewhere (space after if, curly braces around blocks)
  2. IMO, if last_command is null, command_executed should never be invoked, so the invocation should be moved into the first part of the if clause.

@p-e-w
Copy link
Owner

p-e-w commented Nov 15, 2014

This looks good now, but I actually just found that we shouldn't even have this problem in the first place as the "preexec" script apparently has functionality to detect empty commands. For some reason, this is not working though (which is the real bug); I'm currently investigating why.

@p-e-w
Copy link
Owner

p-e-w commented Nov 15, 2014

I found the issue underlying the bug and fixed it in 39b078b (explanation in #377 (comment)). I'm closing this PR since it only masks the real problem which is now solved.

@p-e-w p-e-w closed this Nov 15, 2014
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.

None yet

2 participants