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

async function should stay on the same line #634

Closed
wants to merge 6 commits into from

Conversation

Xiot
Copy link
Contributor

@Xiot Xiot commented Mar 6, 2015

Deals with #630

Sorry about the messy commit. It looks like the code got beautified and removed the tabs that were there.
I haven't figured out how to squash these commits into one yet, but if you really want I can go figure it out.

@Xiot
Copy link
Contributor Author

Xiot commented Mar 6, 2015

I've never used python, so I don't know why the build is failing.

@bitwiseman
Copy link
Member

The build is failing because you're editing the test files directly. Edit the 'test/data/javascript.js' file. Then run make. Then check that in. I should make a build step that enforces this earlier, sorry.

Ensuring no changes visible to git have been made to '' ...
ERROR: Post-build git status check - FAILED.
Git status reported changes to non-git-ignore'd files.
TO REPRO: Run 'git status '.  The use git gui or git diff to see what was changed during the build.
TO FIX: Amend your commit and rebuild. Repeat until git status reports no changes both before and after the build.
OUTPUT FOR 'git status ':
# HEAD detached at FETCH_HEAD
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#   modified:   js/test/beautify-javascript-tests.js
#   modified:   python/jsbeautifier/tests/testjsbeautifier.py
#
no changes added to commit (use "git add" and/or "git commit -a")

@bitwiseman
Copy link
Member

Other than that, your change works seems reasonable, but needs some more tests.

@Xiot
Copy link
Contributor Author

Xiot commented Mar 6, 2015

No worries. I'll revert those files back and put the tests into the proper place.

@bitwiseman
Copy link
Member

Those files will still end up being modified, but they'll be modified by the build. :)

@Xiot
Copy link
Contributor Author

Xiot commented Mar 7, 2015

make testj doesnt seem to work on my machine (Windows 8 x64) it gives me

$ make testj
Testing javascript implementation...
v0.12.0

> js-beautify@1.5.5 test D:\Development\Git Hub\js-beautify
> ./js/test/shell-smoke-test.sh

'.' is not recognized as an internal or external command,
operable program or batch file.
npm ERR! Test failed.  See above for more details.
Makefile:39: recipe for target 'testj' failed
make: *** [testj] Error 1

However I was able to make buildj and run the js tests manually (node .\js\test\node-beautify-tests.js) and they all pass.

Python is still giving me an issue though

$ make buildp
Building python...
Obtaining file:///D:/Development/Git%20Hub/js-beautify/python
    Traceback (most recent call last):
      File "<string>", line 20, in <module>
      File "D:\Development\Git Hub\js-beautify\python\setup.py", line 4, in <module>
        from jsbeautifier.__version__ import __version__
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):

      File "<string>", line 20, in <module>

      File "D:\Development\Git Hub\js-beautify\python\setup.py", line 4, in <module>

        from jsbeautifier.__version__ import __version__

    ----------------------------------------
    Command "python setup.py egg_info" failed with error code 1 in D:\Development\Git Hub\js-beautify\python
Makefile:28: recipe for target 'buildp' failed
make: *** [buildp] Error 1

Although I didn't touch that file at all.

@@ -616,6 +616,10 @@ def handle_start_expr(self, current_token):
# TODO: option space_before_conditional
self.output.space_before_token = True

elif current_token.text == '(' && self.last_type == 'TK_RESERVED' && self.flags.last_word == 'await')
Copy link
Member

Choose a reason for hiding this comment

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

Here's why the python is breaking. This line should look like this:

elif current_token.text == '(' and self.last_type == 'TK_RESERVED' and self.flags.last_word == 'await':

I'll get it.

@bitwiseman
Copy link
Member

👍

@bitwiseman
Copy link
Member

For this windows build error, could you file that as an issue? We don't get much coverage on windows.

@Xiot
Copy link
Contributor Author

Xiot commented Mar 8, 2015

woohoo !
I'll create an issue for the build errors on windows.

@bitwiseman
Copy link
Member

I'm going to flatten this into a single commit and push the change directly.

@Xiot
Copy link
Contributor Author

Xiot commented Mar 8, 2015

I haven't quite figured out how to do that yet. Time for some more learning.

@bitwiseman
Copy link
Member

It's worth learning, but I've got it for this one. Good job on this and with the tests as well.

@bitwiseman bitwiseman closed this in 46cf321 Mar 8, 2015
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.

2 participants