Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Logstash Management] Removing ruby mode from pipeline ACE editor #18807

Conversation

Projects
None yet
3 participants
@ycombinator
Copy link
Contributor

commented May 4, 2018

Resolves #18791.

Prior to this PR, the ACE editor used by the Pipeline Create/Edit form set the editor's mode to ruby. This resulted in ACE making a HTTP GET request to /app/mode-ruby.js. Since that file isn't actually present, the request 404'd and caused an error to appear in the browser's dev console.

Since the primary content of the ACE editor in the Pipeline Create/Edit form is Logstash syntax, I don't think we should use Ruby mode here. Longer-term, we should probably create a custom Logstash mode with its own syntax highlighting rules and such.

For the immediate term, however, this PR simply removes the line setting the mode to ruby, thereby getting rid of the error seen in the dev console.

@ycombinator ycombinator force-pushed the ycombinator:x-pack/management/logstash/remove-ruby-edit-mode branch from 063dd36 to a2eb598 May 4, 2018

@ycombinator ycombinator changed the title Removing line that's causing an error [Logstash Management] Removing ruby mode from pipeline ACE editor May 4, 2018

@elasticmachine

This comment has been minimized.

Copy link

commented May 4, 2018

@justinkambic
Copy link
Contributor

left a comment

LGTM

Question - do we have an issue tracking this enhancement (adding custom LS syntax highlighting)? That sounds like an easy win that would improve the UX.

@ycombinator

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2018

Question - do we have an issue tracking this enhancement (adding custom LS syntax highlighting)? That sounds like an easy win that would improve the UX.

I thought we did but I just did a quick search in https://github.com/elastic/kibana/issues and nothing seemed to turn up. I agree it would be a fairly easy win (there might be some intricacies to worry about with the ruby filter but those could come later too). I just created the issue: #18810.

@ycombinator ycombinator merged commit 5722c84 into elastic:master May 4, 2018

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
kibana-ci Build finished.
Details

ycombinator added a commit that referenced this pull request May 4, 2018

@ycombinator

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2018

Backported to:

@ycombinator ycombinator deleted the ycombinator:x-pack/management/logstash/remove-ruby-edit-mode branch May 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.