Skip to content

Allow opening files at a specific line and column (fixes #5619) #5620

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 6, 2022
Merged

Allow opening files at a specific line and column (fixes #5619) #5620

merged 2 commits into from
Oct 6, 2022

Conversation

danog
Copy link
Contributor

@danog danog commented Oct 4, 2022

Fixes #5619

@danog danog requested a review from a team as a code owner October 4, 2022 18:28
@codecov
Copy link

codecov bot commented Oct 4, 2022 •
edited
Loading

Codecov Report

Merging #5620 (b45b356) into main (3a9eb31) will increase coverage by 0.08%.
The diff coverage is 66.66%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5620      +/-   ##
==========================================
+ Coverage   72.44%   72.52%   +0.08%     
==========================================
  Files          30       30              
  Lines        1673     1678       +5     
  Branches      366      366              
==========================================
+ Hits         1212     1217       +5     
  Misses        398      398              
  Partials       63       63
Impacted Files Coverage Δ
src/node/main.ts 50.00% <25.00%> (ø)
src/node/util.ts 91.80% <100.00%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a9eb31...b45b356. Read the comment docs.

@jsjoeio
Copy link
Contributor

jsjoeio commented Oct 5, 2022

@danog do you plan to add tests? also, can you add to the PR description steps for testing this locally?

@danog
Copy link
Contributor Author

danog commented Oct 5, 2022 •
edited
Loading

Well it's pretty simple to test this locally, just open a workspace and run code-server -r path/to/file.ext:line:col from the workspace directory.
It works pretty much the same as before, the only difference is that now line and column numbers are supported.

@danog
Copy link
Contributor Author

danog commented Oct 5, 2022

Debug builds can be tested using node node_modules/ts-node/dist/bin.js out/node/entry.js -r path/to/file.ext:line:col

@jsjoeio
Copy link
Contributor

jsjoeio commented Oct 5, 2022

Thank you! I tested locally and it worked like a charm!

Screen.Recording.2022-10-05.at.12.39.19.PM.mov

Can you please add some tests?

jsjoeio
jsjoeio reviewed Oct 5, 2022
View reviewed changes
src/node/util.ts
Comment on lines +485 to +493
export const isDirectory = async (path: string): Promise<boolean> => {
try {
const stat = await fs.stat(path)
return stat.isDirectory()
} catch (error) {
return false
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a test for this?

@jsjoeio
Copy link
Contributor

jsjoeio commented Oct 5, 2022

cc @code-asher - i tested locally but would love to get your thoughts too 👍🏼

code-asher
code-asher approved these changes Oct 5, 2022
View reviewed changes
Copy link
Member

@code-asher code-asher left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great. Glad it was so simple! Adding a simple test sounds like a good idea to me as well, can pretty much just copy the isFile one.

jsjoeio
jsjoeio suggested changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@jsjoeio jsjoeio left a comment

Choose a reason for hiding this comment

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

Add a test and then we should be good to merge!

@danog danog requested a review from jsjoeio October 6, 2022 17:15
jsjoeio
jsjoeio approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@jsjoeio jsjoeio left a comment

Choose a reason for hiding this comment

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

Awesome! Looks good to me 👍🏼

@jsjoeio jsjoeio merged commit b562d4a into coder:main Oct 6, 2022
@jsjoeio
Copy link
Contributor

jsjoeio commented Oct 6, 2022

Thanks again for your contribution @danog! 🎉

@danog
Copy link
Contributor Author

danog commented Oct 11, 2022

Thanks for merging, could I have a tag please? :)

@jsjoeio
Copy link
Contributor

jsjoeio commented Oct 11, 2022

@danog by that do you mean tagged in the release notes? :D

@danog
Copy link
Contributor Author

danog commented Oct 12, 2022

I mean a new release, I've been thinking of proposing code-server as an alternative IDE at work, and our workflow heavily relies on the feature introduced in this PR :)

@jsjoeio
Copy link
Contributor

jsjoeio commented Oct 13, 2022

Oh no way, that's exciting! I'm going to see if I can finish #5624 and possibly do #5625 before the next release 🤞🏼

In the meantime, you should be able to see your changes by doing npm install -g code-server@beta (see here) if you use npm to install code-server.

@alanhe421
Copy link

Debug builds can be tested using node node_modules/ts-node/dist/bin.js out/node/entry.js -r path/to/file.ext:line:col

I think URL param-like r support is welcome.

@jsjoeio
Copy link
Contributor

jsjoeio commented Dec 5, 2022

We have an open issue for that: #1964

@code-asher code-asher mentioned this pull request Feb 9, 2023
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jsjoeio jsjoeio jsjoeio approved these changes

@code-asher code-asher code-asher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feat]: --goto flag to open file @ line:column
4 participants
@danog @jsjoeio @alanhe421 @code-asher