Skip to content

[NETBEANS-189] Updates for Sql autocomplete - #2820

Closed
ebresie wants to merge 24 commits into
apache:masterfrom
ebresie:sql_autocomplete
Closed

[NETBEANS-189] Updates for Sql autocomplete#2820
ebresie wants to merge 24 commits into
apache:masterfrom
ebresie:sql_autocomplete

Conversation

@ebresie

@ebresie ebresie commented Mar 20, 2021

Copy link
Copy Markdown
Contributor

This allows autocompletion to be possible when no connection is yet established.

ebresie added 2 commits March 20, 2021 13:05
…ints for possible connection issues (i.e. new driver, new connection, connect - showing existing dialog.
…ints for possible connection issues (i.e. new driver, new connection, connect - showing existing dialog.
@Chris2011

Copy link
Copy Markdown
Contributor

As you already mentioned in your mail, because of the HINTS and FIX, I think so too, that we should separate it. It is out of scope of the ticket and it would be better to have a separated packege/folder whatever where we have SQL Editor related hints and fixes. My 2 cents.

@matthiasblaesing matthiasblaesing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this needs cleanup before it can be merged. Primarily only changes, that should be in master (and are working) should be in this PR. I left some comments inline, which might help.

My concern right now, that this opens a completly new can of worms, because the core assumption ("real" completion only happens when a connection is available) is no broken. I saw some guards added, but yet I managed to get this:

java.lang.NullPointerException
	at org.netbeans.modules.db.sql.editor.completion.SQLCompletionQuery.completeSimpleIdentBasedOnFromClause(SQLCompletionQuery.java:683)
	at org.netbeans.modules.db.sql.editor.completion.SQLCompletionQuery.completeColumnWithDefinedTuple(SQLCompletionQuery.java:501)
	at org.netbeans.modules.db.sql.editor.completion.SQLCompletionQuery.completeSelect(SQLCompletionQuery.java:268)
	at org.netbeans.modules.db.sql.editor.completion.SQLCompletionQuery.doQuery(SQLCompletionQuery.java:211)
	at org.netbeans.modules.db.sql.editor.completion.SQLCompletionQuery.doQuery(SQLCompletionQuery.java:152)
	at org.netbeans.modules.db.sql.editor.completion.SQLCompletionQuery.query(SQLCompletionQuery.java:101)
	at org.netbeans.spi.editor.completion.support.AsyncCompletionTask.run(AsyncCompletionTask.java:198)
	at org.openide.util.RequestProcessor$Task.run(RequestProcessor.java:1418)
	at org.netbeans.modules.openide.util.GlobalLookup.execute(GlobalLookup.java:45)
	at org.openide.util.lookup.Lookups.executeWith(Lookups.java:278)
	at org.openide.util.RequestProcessor$Processor.run(RequestProcessor.java:2033)
Caused: org.openide.util.RequestProcessor$FastItem: task failed due to

from a simple query (completion was invoked at |):

select * from x WHERE |;

Comment thread ide/db.sql.editor/nbproject/project.xml Outdated
Comment thread ide/db.sql.editor/nbproject/project.xml Outdated
Comment thread ide/db.sql.editor/nbproject/project.xml Outdated
} else {
if (quoter == null) {
return null;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks strange (see above).

@ebresie

ebresie commented Mar 21, 2021

Copy link
Copy Markdown
Contributor Author

As you already mentioned in your mail, because of the HINTS and FIX, I think so too, that we should separate it. It is out of scope of the ticket and it would be better to have a separated packege/folder whatever where we have SQL Editor related hints and fixes. My 2 cents.

I have created NETBEANS-5474 Add SQL Hints for use during editing to capture changes associated with adding hints to SQL.

@vitaly-los

Copy link
Copy Markdown

Just interesting is it related commit #2558?
I see some dead commits are from 2019

@ebresie

ebresie commented Mar 21, 2021 via email

Copy link
Copy Markdown
Contributor Author

@ebresie ebresie changed the title Updates for Sql autocomplete Updates for Sql autocomplete WIP Mar 23, 2021
@matthiasblaesing

Copy link
Copy Markdown
Contributor

I suggest to clean this up, remove all unnessary changes, squash and force push. I also notice, that no reference to an issue is given, so the changes are not tracked in jira.

@ebresie ebresie changed the title Updates for Sql autocomplete WIP [NETBEANS-189] Updates for Sql autocomplete WIP Apr 6, 2021
@ebresie

ebresie commented Apr 9, 2021

Copy link
Copy Markdown
Contributor Author

I suggest to clean this up, remove all unnessary changes, squash and force push.

Sorry, I've not done a lot of squashing and force pushes. Any guidance or pointers to that?

Is that something doable within github, Netbeans git, or git command prompt verIon?

@ebresie

ebresie commented Apr 9, 2021

Copy link
Copy Markdown
Contributor Author

no reference to an issue is given, so the changes are not tracked in jira.

As indicate, I've added the ticket reference [NETBEANS-189] to the PR title and the PR is linked on the NETBEANS ticket. Let me know if anything else is needed here.

@matthiasblaesing

Copy link
Copy Markdown
Contributor

I suggest to clean this up, remove all unnessary changes, squash and force push.

Sorry, I've not done a lot of squashing and force pushes. Any guidance or pointers to that?

Is that something doable within github, Netbeans git, or git command prompt verIon?

I think the git integration in netbeans is missing a force-push option, so I would use the git cli client:

https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

After the history is cleaned up, you can do a forced push to the tracking branch on github and the PR will be updated with that.

The intention is here to get a clean baseline (and loose all unintended changes) and should be used with care.

@ebresie

ebresie commented Apr 9, 2021

Copy link
Copy Markdown
Contributor Author

Okay apparently the adding of the notification as part of ide/db.sql.editor/src/org/netbeans/modules/db/sql/editor/completion/SQLCompletionQuery.java broke unit tests. I believe the problem has to do with the dialog opening and waiting for someone to click the button. I assume may need to back this out and/or find a better place to trigger the notification.

@ebresie

ebresie commented Apr 11, 2021

Copy link
Copy Markdown
Contributor Author

I've squashed most of the changes together. So hopefully it's just about ready fro inclusion.

@ebresie ebresie changed the title [NETBEANS-189] Updates for Sql autocomplete WIP [NETBEANS-189] Updates for Sql autocomplete Apr 11, 2021
ebresie added 6 commits April 15, 2021 07:36
…into sql_autocomplete

Reverted layer.xml file to previous version
…into sql_autocomplete

Reverted unneeded layer.xml and Bundle.properties changes
…into sql_autocomplete

Revert changes on project.xml file removing hint and parser dependencies.
@ebresie

ebresie commented Jul 2, 2021

Copy link
Copy Markdown
Contributor Author

I have raised "NETBEANS-5831 Create a SQL Standard Quoter for Use with Connectionless Cases" to accomidate refactoring elements in the above.

@ebarboni ebarboni added this to the 12.5 milestone Jul 6, 2021
@ebarboni

ebarboni commented Jul 6, 2021

Copy link
Copy Markdown
Contributor

hi @ebresie would be good if you can squash and force push to "remove" from history the technical commit.

git rebase -i HEAD~20 (shoud match hash of you commits)
edit the file to replace pick by squash (except for the first)

edit commit message maybe choose the one with Issue number

apply the rebase

if everything is OK
git push --force

ebresie added 2 commits July 16, 2021 10:26
Removed hint dependency
Removed unused variables including hint variable
@ebarboni

Copy link
Copy Markdown
Contributor

change requested moving to 12.6

@ebresie

ebresie commented Jul 23, 2021

Copy link
Copy Markdown
Contributor Author

To reduce the confusion, I would like to close this PR and replaced with a "clean" PR (see #3074) which can be used going forward.

If this is acceptable, then will need the milestones, tags, labels, reviews to be addressed.

@ebresie

ebresie commented Aug 7, 2021

Copy link
Copy Markdown
Contributor Author

Closing this in favor of #3074

@ebresie ebresie closed this Aug 7, 2021
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.

5 participants