Issue 11 print to outputstream #12

bertysentry · 2018-01-31T14:47:29Z

This may conflict with other Pull Requests. If you merge other Pull Request before, I'll have to rebase, but it's okay ;-)

bertysentry · 2018-02-02T21:18:51Z

Updated to remove conflicts

eyalroth · 2018-02-03T21:15:24Z

I hope to get this reviewed next week, I'm just looking to finish up a small re-factor with the compiler first.

This might be annoying but could you perhaps change the PR so only the actual changes are recorded and not all the tab / spaces changes? (I believe the original code uses tabs and not spaces)

bertysentry · 2018-02-04T22:30:12Z

Hi @eyalroth
I'm very sorry, I just realized how much AwkCompilerImpl.java is screwed up. I don't understand why Github is showing it this way. Even the import ... lines (which don't have spaces or tabs indentation) are marked as different! This is weird...
Besides, I'm more of a tab guy than a space guy... ;-)
I edited everything in Eclipse, which doesn't change spaces and tabs.
Anyway, I actually had no significant changes in AwkCompilerImpl.java.

Just:

fixed what Eclipse told me to (prefer Class<?> than just Class and similar issues)
re-implemented your own PR (before you merged it a few days ago)
added a return cg.getJavaClass().getBytes() at the end of the compile(AwkTuples) method, so the caller can do whatever he wants with the bytecode (including loading it directly, without going through a .JAR file)

None of this is really important and can therefore be safely discarded :-)

(edited after realizing my mistake with AwkCompilerImpl.java)

* Fixed the handling of large integers * Fixed the handling of uninitialized variables * Fixed the sprintf() function (and related) to behave like GNU AWK (gawk)

eyalroth

I'm all in favor of adding small re-factoring of small pieces of code while working on a feature / bug (I do it all the time), but I feel it is important to separate these changes from the actual functionality changes as much as possible. Obviously, if a re-factor is in place for the sake of adding / fixing functionality then it should be part of those changes, but if it is unrelated then I would say it should be committed separately (could be in the same PR though). Do you agree?

For the PR itself, there are a few changes missing here in order to fully support the issue:

At Main.java:37 there is the option of starting a script with an alternate output stream - I would expect this to go over to the AwkSetting instance used there instead of changing the entire System.out. I would say that the same should actually be done with System.in, but perhaps that's not part of this PR. In addition, in the same "flow", there is the parsing of the settings from the input, and in case it was requested, the usage of the program will be printed to System.out (AwkParameters.java:169) - again, I would expect it to be printed directly to the custom output stream.
Both jrtSystem and jrtSpawnForOutput methods inside of JRT.java pipe the stdout of a spawned process to the System.out, where it should probably pipe it to the output configured in the settings instead.
The compiler mode still doesn't support the custom output stream correctly. If I'm not mistaken it even has a bug (regardless of this PR). You want me to add the proper handling there or you wanna give it a try (I'll explain the problem if needs be)?
(optional) It is possible to add a custom error stream as well, but really isn't all that important.

P.S.
I would leave the changes to the return type of the compile method aside for this PR; I'm intending to change the compiler a bit in my own PR in a way that would make this irrelevant (only good stuff I promise).

eyalroth · 2018-02-09T17:36:50Z

src/main/java/org/jawk/backend/AVM.java

@@ -1612,7 +1612,7 @@ public void interpret(AwkTuples tuples)
 						// stack[1] = second actual parameter
 						// etc.
 						Address func_addr = position.addressArg();
-						String func_name = position.arg(1).toString();
+						//String func_name = position.arg(1).toString();


Let's make it that we delete code that we don't want to be used anymore and not just comment it out :)

cdrasmussen · 2018-04-08T16:40:57Z

Bertrand, sorry to use this email address but I don't have a better one. I haven't wanted to write this so I have procrastinated for a long time. My heart was in the right place but I've been sick for months. Sometimes it's been hard to simply keep up with the activities of daily life. As much as I'd like to, I simply don't have the cycles to maintain the jawk project. Besides my problems, I think you would be the better maintainer since you have put so much into it. My sincerest apologies, Constantine Rasmussen

…

On Wed, Jan 31, 2018, 9:47 AM Bertrand Martin ***@***.***> wrote: This may conflict with other Pull Requests. If you merge other Pull Request before, I'll have to rebase, but it's okay ;-) ------------------------------ You can view, comment on, or merge this pull request online at: #12 Commit Summary - Improved .gitignore - Added support for arbitrary *PrintStream* output (instead of System.out) File Changes - *M* .gitignore <https://github.com/hoijui/Jawk/pull/12/files#diff-0> (7) - *M* src/main/java/org/jawk/Awk.java <https://github.com/hoijui/Jawk/pull/12/files#diff-1> (8) - *M* src/main/java/org/jawk/Main.java <https://github.com/hoijui/Jawk/pull/12/files#diff-2> (2) - *M* src/main/java/org/jawk/backend/AVM.java <https://github.com/hoijui/Jawk/pull/12/files#diff-3> (14) - *M* src/main/java/org/jawk/backend/AwkCompiler.java <https://github.com/hoijui/Jawk/pull/12/files#diff-4> (2) - *M* src/main/java/org/jawk/backend/AwkCompilerImpl.java <https://github.com/hoijui/Jawk/pull/12/files#diff-5> (144) - *M* src/main/java/org/jawk/ext/CoreExtension.java <https://github.com/hoijui/Jawk/pull/12/files#diff-6> (7) - *M* src/main/java/org/jawk/intermediate/AwkTuples.java <https://github.com/hoijui/Jawk/pull/12/files#diff-7> (49) - *M* src/main/java/org/jawk/jrt/JRT.java <https://github.com/hoijui/Jawk/pull/12/files#diff-8> (51) - *M* src/main/java/org/jawk/jrt/VariableManager.java <https://github.com/hoijui/Jawk/pull/12/files#diff-9> (1) - *M* src/main/java/org/jawk/util/AwkParameters.java <https://github.com/hoijui/Jawk/pull/12/files#diff-10> (5) - *M* src/main/java/org/jawk/util/AwkSettings.java <https://github.com/hoijui/Jawk/pull/12/files#diff-11> (27) Patch Links: - https://github.com/hoijui/Jawk/pull/12.patch - https://github.com/hoijui/Jawk/pull/12.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#12>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADAU9AqP6dm1yeeOL9IFZrqRCB9YJv6nks5tQH0BgaJpZM4R0Fcd> .

bertysentry · 2018-04-09T13:57:35Z

Hi @cdrasmussen

I'm very sorry to hear about your health problems. I hope you'll get better soon!

I'd be happy to handle the maintenance of Jawk. As he was involved in Jawk maintenance too, maybe @eyalroth would like to handle that. @eyalroth ?

Thanks,
-- Bertrand

…g-int to master * commit 'b5a9ffa0ade0d3dfb88eef0e38b7b660e13affca': Fixed an issue with sprintf("%g") which does not behave the same way in Java and in C (and therefore in AWK) issue #12 Added support for arbitrary *PrintStream* output (instead of System.out) Improved .gitignore

hoijui · 2018-04-27T11:44:46Z

i am a bit confused.. eyalroth is already the maintainer since some months now.
if someone is unhappy for not getting commits into the repo due to lazy or too restrictive maintainer(s) (what aeh.. whom you all looking at?!!), ask for maintainer permissions. :-)
to me he seems like eyalroth is doing a very good job at the moment.

bertysentry and others added 3 commits January 27, 2018 13:18

Improved .gitignore

eb920dc

Added support for arbitrary *PrintStream* output (instead of System.out)

a1523a0

Merge branch 'master' into issue-11-print-to-outputstream

b60fdfa

bertysentry mentioned this pull request Feb 6, 2018

Issue #13 fix display long int, sprintf, uninitialized variables #14

Closed

bertysentry referenced this pull request in sentrysoftware/Jawk Feb 6, 2018

issue #12

0e43e73

* Fixed the handling of large integers * Fixed the handling of uninitialized variables * Fixed the sprintf() function (and related) to behave like GNU AWK (gawk)

eyalroth requested changes Feb 9, 2018

View reviewed changes

bertysentry closed this Jul 20, 2023

bertysentry deleted the issue-11-print-to-outputstream branch July 20, 2023 16:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 11 print to outputstream #12

Issue 11 print to outputstream #12

bertysentry commented Jan 31, 2018

bertysentry commented Feb 2, 2018

eyalroth commented Feb 3, 2018

bertysentry commented Feb 4, 2018 •

edited

Loading

eyalroth left a comment •

edited

Loading

eyalroth Feb 9, 2018

cdrasmussen commented Apr 8, 2018 via email

bertysentry commented Apr 9, 2018

hoijui commented Apr 27, 2018

Issue 11 print to outputstream #12

Issue 11 print to outputstream #12

Conversation

bertysentry commented Jan 31, 2018

bertysentry commented Feb 2, 2018

eyalroth commented Feb 3, 2018

bertysentry commented Feb 4, 2018 • edited Loading

eyalroth left a comment • edited Loading

Choose a reason for hiding this comment

eyalroth Feb 9, 2018

Choose a reason for hiding this comment

cdrasmussen commented Apr 8, 2018 via email

bertysentry commented Apr 9, 2018

hoijui commented Apr 27, 2018

bertysentry commented Feb 4, 2018 •

edited

Loading

eyalroth left a comment •

edited

Loading