Skip to content

Support advance properteis for sql hint#3095

Closed
gzzhanghao wants to merge 1 commit intocodemirror:masterfrom
gzzhanghao:master
Closed

Support advance properteis for sql hint#3095
gzzhanghao wants to merge 1 commit intocodemirror:masterfrom
gzzhanghao:master

Conversation

@gzzhanghao
Copy link
Copy Markdown
Contributor

Add support for advance properties (displayText for example) for sql-hint addon.

Here is an example for the new feature:

options.tables = [{
  text: 'table-name',
  displayText: 'table-name | comment',
  columns: [{
    text: 'column-name',
    displayText: 'column-name | column-type'
  }]
}]

@marijnh
Copy link
Copy Markdown
Member

marijnh commented Feb 20, 2015

You are changing the table format in an incompatible, breaking way. That is not acceptable.

@gzzhanghao
Copy link
Copy Markdown
Contributor Author

@marijnh The patch is backward compatible, the original table format is supported as well.

marijnh added a commit that referenced this pull request Feb 20, 2015
@marijnh
Copy link
Copy Markdown
Member

marijnh commented Feb 20, 2015

Fair enough. Merged as 8f13aa6 and simplified and tested in 1eb72e8.

@marijnh marijnh closed this Feb 20, 2015
if (typeof item == 'string') {
return item;
}
return item.text;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't this be item.displayText?
Let's suppose I have a table object {columns:{....}, text: '{{table1}}', displayText:'table1'}
Then when I type 'ta' it does not return me anything. Am I missing something?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be item.displayText?

No, this is used to get the actual completion text, not the label.

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.

3 participants