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

coala-json should give relpaths of files #1593

Closed
sils opened this issue Feb 19, 2016 · 10 comments
Closed

coala-json should give relpaths of files #1593

sils opened this issue Feb 19, 2016 · 10 comments

Comments

@sils
Copy link
Member

sils commented Feb 19, 2016

IMO we should output relative paths in all scenarios. (This is needed for coala-html to work based on coala-json.)

@sils
Copy link
Member Author

sils commented Feb 21, 2016

commit was reverted @tushar-rishav you're still assigned

@AbdealiLoKo
Copy link
Contributor

Why was the commit reverted ?
And also, I dislike relative paths. Because when I run coala on something like /tmp/abcde it gives a lot of ../../../../../. Although this is a rare case, it was a little irritating when i saw this somewhere.

Wouldn't it be possible to track whether the user gave a relative path for the file glob and decide on abs or rel based on that ?
OR - would it be possible for coala-html to simply remove the extra part for its current path ?

@sils
Copy link
Member Author

sils commented Feb 22, 2016

it broke because the file_dict still used absolute paths so coala couldn't
get the file contents anymore

2016-02-22 13:03 GMT+01:00 AbdealiJK notifications@github.com:

Why was the commit reverted ?
And also, I dislike relative paths. Because when I run coala on something
like /tmp/abcde it gives a lot of ../../../../../. Although this is a
rare case, it was a little irritating when i saw this somewhere.

Wouldn't it be possible to track whether the user gave a relative path for
the file glob and decide on abs or rel based on that ?
OR - would it be possible for coala-html to simple remove the extra part
for its current path ?


Reply to this email directly or view it on GitHub
#1593 (comment)
.

@sils
Copy link
Member Author

sils commented Feb 22, 2016

we already use relative paths for the output of filenames in
consoleinteraction, my thought was to switch to relative paths overall
possibly. Maybe provide a setting for it? :/

2016-02-22 13:05 GMT+01:00 Lasse Schuirmann lasse.schuirmann@gmail.com:

it broke because the file_dict still used absolute paths so coala couldn't
get the file contents anymore

2016-02-22 13:03 GMT+01:00 AbdealiJK notifications@github.com:

Why was the commit reverted ?
And also, I dislike relative paths. Because when I run coala on something
like /tmp/abcde it gives a lot of ../../../../../. Although this is a
rare case, it was a little irritating when i saw this somewhere.

Wouldn't it be possible to track whether the user gave a relative path
for the file glob and decide on abs or rel based on that ?
OR - would it be possible for coala-html to simple remove the extra part
for its current path ?


Reply to this email directly or view it on GitHub
#1593 (comment)
.

@AbdealiLoKo
Copy link
Contributor

Ah, ^ @sils1297 I disliked the ConsoleInteraction using relpaths then - as it gave me the ../../../ etc at some point of time.
But again, thats a rare case.

Why exactly does coala-html need this ? If it's using json, can't we simply show the path in json ouput ?

@sils
Copy link
Member Author

sils commented Feb 22, 2016

so if you do the json dump the html/js page uses paths to get file
contents, if you then zip it and upload it somewhere to be served
statically it won't work if you use abspaths in the json dump

2016-02-22 13:09 GMT+01:00 AbdealiJK notifications@github.com:

Ah, ^ @sils1297 https://github.com/sils1297 I disliked the
ConsoleInteraction using relpaths then - as it gave me the ../../../ etc
at some point of time.
But again, thats a rare case.

Why exactly does coala-html need this ? If it's using json, can't we
simply show the path in json ouput ?


Reply to this email directly or view it on GitHub
#1593 (comment)
.

@AbdealiLoKo
Copy link
Contributor

Maybe have a --relpath in coala-json ?

On Mon, Feb 22, 2016 at 5:43 PM, Lasse Schuirmann notifications@github.com
wrote:

so if you do the json dump the html/js page uses paths to get file
contents, if you then zip it and upload it somewhere to be served
statically it won't work if you use abspaths in the json dump

2016-02-22 13:09 GMT+01:00 AbdealiJK notifications@github.com:

Ah, ^ @sils1297 https://github.com/sils1297 I disliked the
ConsoleInteraction using relpaths then - as it gave me the ../../../ etc
at some point of time.
But again, thats a rare case.

Why exactly does coala-html need this ? If it's using json, can't we
simply show the path in json ouput ?


Reply to this email directly or view it on GitHub
<
#1593 (comment)

.


Reply to this email directly or view it on GitHub
#1593 (comment)
.

@tushar-rishav
Copy link
Member

Yes. It's possible to convert abspath in json_dump file from coala-json
to relpath. In that case we will have to iterate through all the file paths
in affected_code and change it to relative paths and finally save the
changes to json_dump
On 22 Feb 2016 17:33, "AbdealiJK" notifications@github.com wrote:

Why was the commit reverted ?
And also, I dislike relative paths. Because when I run coala on something
like /tmp/abcde it gives a lot of ../../../../../. Although this is a
rare case, it was a little irritating when i saw this somewhere.

Wouldn't it be possible to track whether the user gave a relative path for
the file glob and decide on abs or rel based on that ?
OR - would it be possible for coala-html to simple remove the extra part
for its current path ?


Reply to this email directly or view it on GitHub
#1593 (comment)
.

@AbdealiLoKo
Copy link
Contributor

@tushar-rishav it's quite a bit simpler - We just add an if in coalib.output.JSONEncoder to catch Result classes and special handle them

@tushar-rishav
Copy link
Member

Makes sense. Let me try.
On 22 Feb 2016 18:06, "AbdealiJK" notifications@github.com wrote:

@tushar-rishav https://github.com/tushar-rishav it's quite a bit
simpler - We just add an if in coalib.output.JSONEncoder to catch Result
classes and special handle them


Reply to this email directly or view it on GitHub
#1593 (comment)
.

@AbdealiLoKo AbdealiLoKo changed the title SourcePosition should do a relpath on the file coala-json should give relpaths of files Feb 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants