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.

Sign up for GitHub

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

Jump to bottom

39 add option to only show specific lines #44

Merged
merged 13 commits into from
Oct 26, 2023
Merged

39 add option to only show specific lines #44

merged 13 commits into from
Oct 26, 2023

Conversation

myousuffazal
Copy link
Contributor

@myousuffazal myousuffazal commented Oct 17, 2023 •
edited by malberts
Loading

For #39

@myousuffazal
Front-end side of working done
0c6209f 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Merge branch 'master' into 39-add-option-to-only-show-specific-lines
e41e78f 
# Conflicts:
#	extension.json
@myousuffazal
Fix
8b2cc78
@myousuffazal
Completed
dec3100 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Fix
fd9caa3 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Changes
89e97b7 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal myousuffazal linked an issue Oct 17, 2023 that may be closed by this pull request
Add option to only show specific lines #39
Closed
@myousuffazal
Copy link
Contributor Author

Attaching images for pictorial reference to the changes done.

image
image
image
image

@malberts
Copy link
Contributor

malberts commented Oct 18, 2023 •
edited
Loading

I'm still reviewing the code, but it is not working when I use a Python file.

Here is what it looks like with specificLines:
image

However, PHP seems to be fine:
Screenshot_20231018_112523

malberts
malberts requested changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@malberts malberts left a comment

Choose a reason for hiding this comment

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

Marked as changes needed due to Python files not working.

resources/code-renderer.css Outdated Show resolved Hide resolved
tests/Integration/Domain/CodeRendererTest.php Outdated Show resolved Hide resolved
src/Domain/ContentRenderer/CodeRenderer.php Outdated Show resolved Hide resolved
JeroenDeDauw
JeroenDeDauw reviewed Oct 18, 2023
View reviewed changes
resources/code-specific-lines.js
else {
showLines.push( Number( currentRange ) );
}
});
Copy link
Member

Choose a reason for hiding this comment

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

This could go into a nice pure-function with full unit-test coverage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified

@myousuffazal
Fixed for other Language Code Files
8b1a1dc 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Test Addition
880aaf3 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Reversion
dd1df7e 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Normalization of Variable
e6b5dd7 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Changes
678badc 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Fixes
f31b4c5 
Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Copy link
Contributor Author

Fixed for other langugages

@myousuffazal myousuffazal requested a review from malberts October 25, 2023 14:23
@JeroenDeDauw
Copy link
Member

Has conflicts with master. Needs rebase or merge

@myousuffazal
Merge branch 'master' into 39-add-option-to-only-show-specific-lines
f12a7d7 
# Conflicts:
#	src/Domain/ContentRenderer/CodeRenderer.php
#	src/Domain/ContentRenderer/RendererConfig.php
#	src/UseCases/Embed/EmbedRequest.php
#	tests/Integration/Domain/CodeRendererTest.php
#	tests/Unit/Domain/WikiContentRendererFactoryTest.php
#	tests/Unit/UseCases/Embed/EmbedUseCaseTest.php

Signed-off-by: Muhammad Yousuf Fazal <[email protected]>
@myousuffazal
Copy link
Contributor Author

Conflicts resolved.

@JeroenDeDauw JeroenDeDauw merged commit 25df96d into master Oct 26, 2023
8 checks passed
@JeroenDeDauw JeroenDeDauw deleted the 39-add-option-to-only-show-specific-lines branch October 26, 2023 11:55
@myousuffazal myousuffazal restored the 39-add-option-to-only-show-specific-lines branch October 27, 2023 07:44
@malberts malberts mentioned this pull request Oct 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malberts malberts malberts requested changes

@JeroenDeDauw JeroenDeDauw JeroenDeDauw left review comments

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

Successfully merging this pull request may close these issues.

Add option to only show specific lines
3 participants
@myousuffazal @malberts @JeroenDeDauw