-
Notifications
You must be signed in to change notification settings - Fork 101
Dbext ora extensions #25
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
base: master
Are you sure you want to change the base?
Conversation
downloaded in 2021 from www.vim.org - as is
Several general fixes for UTF8 - dbext_dbi_debug - changed spelling to use 'g:' prefix - perl prologue - explicitly asking perl to use UTF8 in both source code and input/output encoding - display of non-ASCII results (importang for UTF8!) of query is added (v26 had $val =~tr/\x80-\xFF/ /; which replaced all non-ASCII things with blanks)! - db_print_results - fixed bug if empty line happened in the non-expected place - FuzzyFinder badly interact with dbext in that it creates global variable FuzzyFinderMode 2 New oracle+DBI specific extensions: - DBXtractDdl - new command which is only implemented for oracle/DBI driver - DBEnableSrvOut - DBDisableSrvOut - new commands to interact with dbms_output from oracle (enable/disable). dbms_output is enabled by default - empty lines in output are forced to stay - otherwise output of DDL is not useable
Problem: dynamically linking perl is broken Solution: Fix all issues This is a combination of several commits: 1) Fix if_perl.xs not being able to build on all versions of Perl (5.30) This fixes the dynamic builds of Perl interface. The Perl interface file previously had to manually copy and paste misc inline functions verbatim from the Perl headers, because we defined `PERL_NO_INLINE_FUNCTIONS` which prevents us form getting some function definitions. The original reason we defined it was because those inline functions would reference Perl functions that would cause linkage errors. This is a little fragile as every time a new version of Perl comes out, we inevitably have to copy over new versions of inline functions to our file, and it's also easy to miss updates to existing functions. Instead, remove the `PERL_NO_INLINE_FUNCTIONS` define, remove the manual copy-pasted inline functions. Simply add stub implementations of the missing linked functions like `Perl_sv_free2` and forward them to the DLL version of the function at runtime. There are only a few functions that need this treatment, and it's a simple stub so there is very low upkeep compared to copying whole implementations to the file. Also, fix the configure script so that if we are using dynamic linkage, we don't pass `-lperl` to the build flags, to avoid accidental external linkage while using dynamic builds. This is similar to how Python integration works. 2) Fix GIMME_V deprecation warnings in Perl 5.38 Just use GIMME_V, and only use GIMME when using 5.30 to avoid needing to link Perl_block_gimme. We could provide a stub like the other linked functions like Perl_sv_free2, but simply using GIMME is the simplest and it has always worked before. 3) Fix Perl 5.38 issues Fix two issues: 3.1. Perl 5.38 links against more functions in their inline headers, so we need to stub them too. 3.2. Perl 5.38 made Perl_get_context an inline function, but *only* for non-Windows build. Fix that. Note that this was happening in Vim currently, as it would build, but fail to run Perl code at runtime. 4) Fix Perl 5.36/5.38 when thread local is used Perl 5.36 introduced using `_Thread_local` for the current context, which causes inline functions to fail. Create a stub `PL_current_context` thread local variable to satisfy the linker for inlined functions. Note that this is going to result in a different `PL_current_context` being used than the one used in the library, but so far from testing it seems to work. 5) Add docs for how to build Perl for dynamic linking to work closes: #12827 closes: #12914 Signed-off-by: Christian Brabandt <[email protected]> Co-authored-by: Yee Cheng Chin <[email protected]>
autoload/dbext_dbi.vim
Outdated
# ans, 2022-04-08 - allow to print from dbms_output empty lines | ||
my @lines = (); | ||
if (!defined ($line_txt) || length($line_txt) == 0) { | ||
@lines = (""); | ||
} else { | ||
@lines = split("\n", $line_txt); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indenting with the rest of the file.
The file should be 4 space expanded indenting.
I am surprised it is off given this line in the file (specifically for Vim)
" vim:fdm=marker:nowrap:ts=4:expandtab:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably I manually ("unconsciously") removed extra spaces in that case (my "standard" ts = 2 usually)
#$temp_length = length((defined($col)?$col:"")); | ||
# For some reason the above can sometimes return the wrong length. | ||
# The following two lines fix that problem | ||
my $xstr = $col; | ||
$temp_length = length((defined($col)?$xstr:"")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm but this not comes from me. I took that other from v26, downloaded in 2021 from vim scripts site.
Looking on that precisely I see no real reason for this change. As far as I can tell lines 1434+1435 doing exactly the same as line 1431 in the past?
(Well, single possible reason for difference which I can imagine is that encoding of the string is changed internally due to assignment but not the byte content of string - and it would be relevant only for some non-ascii bytes... But that's just a theory)
autoload/dbext_dbi.vim
Outdated
#### THIS IS MAGIC !!!!!!! | ||
#### THIS_IS_MAGIC !!!!!!! | ||
#### THIS IS MAGIC !!!!!!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is magic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was too excited of being able to figure out, that decode('utf8', $str)
solves the problem with dbms_output non-ASCII chars being wrongly encoded ))) Cleaned up this comments a bit as well as extracted example referenced which reproduces a problem as separate file on github (hope it is available publicly):
autoload/dbext_dbi.vim
Outdated
db_debug("skip <enable> of already enabled server output"); | ||
} | ||
else { | ||
$conn_local->func( 1000000, 'dbms_output_enable' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense for this to be a variable so that it can be customized easily?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would make perfectly sense but I have to find time yet to work on it. Next 2-3 weeks are quite tough on my side so I push now to get at least quick changes you requested done.
@@ -1,4 +1,4 @@ | |||
*dbext.txt* For Vim version 7.0. Last change: 2016 Jan 03 | |||
*dbext.txt* For Vim version 7.0. Last change: 2017 Nov 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a section at the bottom of this file around:
17.13 DBI Tutorial dbext-tutorial-dbi
Wishes shows this feature in action.
A command you execute and the output you get to see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some very "microscopic" documentation please have a look.
a92bdf9
to
ae3533f
Compare
ae3533f
to
8458ff0
Compare
@dfishburn, done so far with "quick wins". Next 3 weeks will have very less time - perhaps only for very small things (like improve docu a bit if you will point to some specfic place). TODO/still open:
Typically it has to be taken over from NLS_LANG environment variable. But because of some quirks it has not worked for me on cygwin with oracle instant client. Yet to try out regularly on linux, IMO not necessary as global default. But for cygwin case at least has to be as well made configurable. |
TODO/continuation:
|
Several general fixes for UTF8 and 2 new oracle+DBI specific extensions