Skip to content

Conversation

@bpurinton
Copy link
Contributor

@bpurinton bpurinton commented Oct 18, 2024

Resolves #9


Important

Adds a "View Source" button to the development toolbar, allowing users to view the source code of the current page.

  • Behavior:
    • Adds "View Source" button to the development toolbar in middleware.rb.
    • Uses view-source: URL scheme to open the source code of the current page.
  • Configuration:
    • Updates README.md to include "View Source" link in configuration example.

This description was created by Ellipsis for 52a8103. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 756b4af in 15 seconds

More details
  • Looked at 47 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. README.md:27
  • Draft comment:
    The path for "View Source" should not be static "/to-do". It should dynamically use the current request URL, similar to how it's implemented in the middleware.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is about a change made in the diff, specifically the addition of a static path for 'View Source'. The suggestion to make it dynamic could be a valid code quality improvement if the intention is to always link to the current request URL. However, without more context on the intended functionality, it's speculative whether this change is necessary.
    The comment assumes that the path should be dynamic without evidence that this is the intended functionality. It could be that a static path is intentional for a specific reason.
    The suggestion to make the path dynamic could improve the functionality if the goal is to link to the current request URL, but without clear evidence that this is the intended behavior, the comment remains speculative.
    Delete the comment as it is speculative and lacks strong evidence that a change is required.

Workflow ID: wflow_1kGg83LzRwVmISOh


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 7056fd2 in 10 seconds

More details
  • Looked at 49 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. lib/dev_toolbar/middleware.rb:82
  • Draft comment:
    The request parameter is unused in the toolbar_links method and should be removed.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_LT0ImMG6QgdSfC3G


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 2c4d7d1 in 15 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. lib/dev_toolbar/middleware.rb:82
  • Draft comment:
    The toolbar_links method has an unused request parameter. Remove it to avoid confusion.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment correctly identifies that the request parameter was unused and suggests its removal, which is exactly what the diff shows was done. This indicates that the comment is valid and relevant to the change made.
    The comment might be considered unnecessary since the change has already been made, and the comment does not provide additional value beyond what the diff already shows.
    Even though the change has been made, the comment correctly identifies the reason for the change, which can be useful for understanding the context of the modification.
    The comment is valid as it correctly identifies the reason for the change made in the diff. It should be kept as it provides context for the modification.

Workflow ID: wflow_yvpLwf6kfgy8Gbyv


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on 349f8d7 in 18 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_ztR51tRRyrZTDsiN


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 52a8103 in 14 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. lib/dev_toolbar/middleware.rb:85
  • Draft comment:
    The view-source: URL scheme may not be supported by all browsers, leading to inconsistent behavior. Consider adding a fallback or handling unsupported browsers.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment addresses a potential issue with browser compatibility for the 'view-source:' URL scheme. This is a valid concern as not all browsers may support this scheme, which could lead to inconsistent behavior. The comment suggests considering a fallback or handling unsupported browsers, which is a reasonable suggestion for improving code robustness.
    The comment could be seen as speculative since it assumes that some browsers do not support the 'view-source:' scheme without providing specific examples. However, it is a known issue that not all browsers handle this scheme the same way.
    Even though the comment is somewhat speculative, it highlights a potential issue that could affect the functionality of the code across different browsers, which is important to consider.
    Keep the comment as it highlights a potential issue with browser compatibility that could affect the functionality of the code.

Workflow ID: wflow_8ZyILq0Dda10QrsC


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@bpurinton bpurinton marked this pull request as draft October 18, 2024 22:26
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.

Add a button for "View Source"

2 participants