mdBook

mdBook copied to clipboard

In this PR, I have added support for the new LinkType::IncludeNoIndent. This feature can be utilized with the Anchor syntax, such as {{#include file.rs:anchor_name:-no-indent}}, or with the Range syntax, {{#include file.rs::20:-no-indent}}.

Parsing is done in the same way as with the Range(LineRange) type. This means that if we use {{#include file.rs::20: 30}}, the command will not work until the whitespace between 20 and 30 is removed, resulting in 20:30. The same applies to the -no-indent command, {{#include file.rs::20: -no-indent}} will not work, while {{#include file.rs::20:-no-indent}} does.

The parsing process begins in the parse_include_path function where we check for the presence of the -no-indent command. Based on this, we return LinkType::IncludeNoIndent or only LinkType::Include.

If we have returned LinkType::IncludeNoIndent, the take_format_remove_indent function will find the part of the code closest to the left margin and move it to the left by that much space. The format of the code remains the same, with only the indentation being removed.

{{#include file.rs::20:30}}

{{#include file.rs::20:30:-no-indent}}

{{#include ../../../examples/providers/src/lib.rs:setup_test_blockchain}}

{{#include ../../../examples/providers/src/lib.rs:setup_test_blockchain:-no-indent}}

https://github.com/rust-lang/mdBook/issues/1601 https://github.com/FuelLabs/fuels-rs/issues/388

ehuss Hello, Hello :)

hi thanks. just give us some time we will review this :)

Hi, I'm also having this problem - did this get merged?

Is the idea to remove all indentation from code snippets or only "extra" indentation? Hopefully, the latter.

Hi, I'm also having this problem - did this get merged?

Is the idea to remove all indentation from code snippets or only "extra" indentation? Hopefully, the latter.

No.

Hi, I'm also having this problem - did this get merged? Is the idea to remove all indentation from code snippets or only "extra" indentation? Hopefully, the latter.

No.

Do you mean that it will remove all indentation, ie.

   fn test() {
      cat.foo();
   }

Becomes:

fn test() {
cat.foo();
}

Or (what I'm hoping):

fn test() {
   cat.foo();
}

Hi, I'm also having this problem - did this get merged? Is the idea to remove all indentation from code snippets or only "extra" indentation? Hopefully, the latter.

No.

Do you mean that it will remove all indentation, ie.

   fn test() {
      cat.foo();
   }

Becomes:

fn test() {
cat.foo();
}

Or (what I'm hoping):

fn test() {
   cat.foo();
}

Second option. What you need too :)

Just pinging this thread to say that I've also run into this problem and would find this PR really useful when merged

Do we expect this to get merged soon? If not I may create a fork to use in the meantime, this would solve some headaches.

If you can test it and ensure that it works as ~~indented~~ intended, it will make it easier for us to review & merge :)

🤣 I'll try to find some time.

I would prefer to avoid adding an option for this. I would expect there to be one true behavior.

There's also PRs and issues about indentation (#1564, #1565, #1718). Whatever solution is taken, I would like to see a clear description of the issue and why a particular solution was chosen. This particular PR doesn't have much of a description.

Another thing to consider is the behavior around hidden lines.

@ehuss @sanity @Dylan-DPC I added some description! Can you take a look?

Can you respond to the questions raised in https://github.com/rust-lang/mdBook/pull/1856#issuecomment-1348532278? Would it be possible to avoid requiring an option to determine the indentation behavior? Why choose this behavior over the other PRs?

I think that in these PR (https://github.com/rust-lang/mdBook/issues/1564, https://github.com/rust-lang/mdBook/pull/1565, https://github.com/rust-lang/mdBook/pull/1718) indent commands are not optional. What will we do if the user likes the way MdBook currently formats the text? That's why I added a new option that is easy to use. There is an option to expand Include(PathBuf, RangeOrAnchor, "/ indent/no_indent /").

@ehuss

@salka1988 - thank you for working on this :)

Opt-in makes sense from the perspective of not breaking existing code, although for me I would prefer a way to set this flag globally for all {{#include}}s rather than for each individually.

Still, this would be a big improvement over not having this functionality.

This means that if we use {{#include file.rs::20: 30}}, the command will not work until the whitespace between 20 and 30 is removed, resulting in 20:30. The same applies to the -no-indent command, {{#include file.rs::20: -no-indent}} will not work, while {{#include file.rs::20:-no-indent}} does.

I understand there may be internal reasons for it, this is definitely going to generate bug reports without a helpful error message :)

Just to add my opinion as a user here. The behaviour described in #1564 and #1626 is clearly incorrect so shouldn't require an option on the part of the user to get correct behaviour. The PR at #1718 implements exactly the solution that #1626 describes. So, if it were up to me, (which it clearly isn't :smile: ) I'd do the following:

• Close #1626 as a duplicate of #1564; the latter is just a subset of the case described in the former
• Close this PR in favour of #1718; as mentioned that PR requires no options and also includes test cases.

Any progress on this PR?