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

Remove unused imports, and deprecated function usage #18663

Merged
merged 3 commits into from
Aug 10, 2021
Merged

Conversation

kdb424
Copy link
Contributor

@kdb424 kdb424 commented Aug 9, 2021

Compiler warned of unused imports.

@kdb424 kdb424 changed the title clean up imports and slice to remove delete Remove unused imports, and depricated function usage. Aug 9, 2021
@kdb424 kdb424 changed the title Remove unused imports, and depricated function usage. Remove unused imports, and deprecated function usage. Aug 9, 2021
@@ -835,15 +835,15 @@ proc docstringSummary(rstText: string): string =
result = rstText.substr(2).strip
var pos = result.find('\L')
if pos > 0:
result.delete(pos, result.len - 1)
result = result[0 .. pos]
Copy link
Member

Choose a reason for hiding this comment

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

buggy patch and allocates un-necessarily

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake on the allocation. Slices can be done in place, so my mistake on that. Can you elaborate on "buggy"? It should achieve the same result as far as I can tell.

Copy link
Member

Choose a reason for hiding this comment

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

off by 1 error, try it

Copy link
Contributor Author

@kdb424 kdb424 Aug 9, 2021

Choose a reason for hiding this comment

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

Thank you. I came from python, and read up on the differences. Thank you for your patience.
EDIT: I'm reverting this change as even after getting the correct slice result 0 ..< pos I still could not avoid the assignment. Will simplify the PR to only deleting unused imports.

Copy link
Member

Choose a reason for hiding this comment

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

why not simply use result.setLen(pos - 1) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does in fact do that both in place, and do the same thing. Thank you again for helping me learn about these things, and being patient.

@kdb424 kdb424 changed the title Remove unused imports, and deprecated function usage. Remove unused imports Aug 9, 2021
@kdb424 kdb424 changed the title Remove unused imports Remove unused imports, and depricated function usage Aug 9, 2021
@timotheecour timotheecour changed the title Remove unused imports, and depricated function usage Remove unused imports, and deprecated function usage Aug 9, 2021
@Araq Araq merged commit 31fc0f9 into nim-lang:devel Aug 10, 2021
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
* clean up imports and slice to remove delete

* revert buggy code

* Replace "delete" with setlen to remove depreciation warning
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.

3 participants