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

Pelican should print a warning when encountering a .md file while markdown is not installed #1868

Open
Psycojoker opened this issue Nov 18, 2015 · 36 comments
Milestone

Comments

@Psycojoker
Copy link

Hello,

Currently, pelican will skip .md files if the "markdown" python packge is not installed. This result in a WARNING: No valid files found in content." which is extremely confusing and has made me lost 30min of debugging.

A better behavior would be to print a warning because there isn't a situation where a user would add a .md file to his content/ while not wanting it to be parsed.

Kind regards,

@avaris
Copy link
Member

avaris commented Nov 18, 2015

Even though the documentation is extremely clear on that?

@Psycojoker
Copy link
Author

Well, the python adage as always been "explicit is better than implicit", in this case you have no indication that there might be a problem, it just doesn't work, it's a bit like saying "oh you need to set up this environment variable for this to be working", it wipes out the possibility of self discovery which is, for me, a bad practice, documented or not.

@ingwinlu
Copy link
Contributor

@Psycojoker if you need 30min to look up the documentation and find a solution then that is a documentation problem. However I think it is reasonably good at explaining the issue.

As for more warnings regarding the content when no Markdown Parser is available:
Currently pelican is only looking for files of certain extensions when their reader is activated. If no markdown is available, .md files are not even looked at and are 'invisible' to pelican.
I think that is great, it is one of the few occasions where pelican is actually abstracted well and can easily be extended or modified to only use a subset of readers that is needed.

Your suggestion would have implications for everyone who does not want to use markdown. Actually scanning the content directory for .md files when just using .rst could be a huge time waste for large sites with lots of content files.

A better alternative could be to print the active Readers and their supported file endings when starting up pelican, but again, this is probably not information you want to see on every run and would rather hide it to the debug menu in case 'no valid content' warning is not triggered.

Your usage of fat formatting and the way you phrase your comments makes you sound like a prig. When I first read it I wanted to react similarly to @avaris.

@Psycojoker
Copy link
Author

A better alternative could be to print the active Readers and their supported file endings when starting up pelican, but again, this is probably not information you want to see on every run and would rather hide it to the debug menu in case 'no valid content' warning is not triggered.

If find the "only active when 'no valid content' case is encounter" to be a good middle ground. In my head you were using something like os.walk which would have allowed this operation without to much overhead since you would have been scanning all file names.

Your usage of fat formatting and the way you phrase your comments makes you sound like a prig. When I first read it I wanted to react similarly to @avaris.

Sorry, the opposite was actually intent, as in "this is only my opinion which my not be the best opinion", I should have made it clearer to avoid ambiguity, my apologies :/

@avaris
Copy link
Member

avaris commented Nov 19, 2015

We use os.walk to scan files, but we filter it to the ones pelican can process. That is obtained from the active readers. And missing dependencies isn't the only way to disable a reader (see READERS config).

Also, people might put .md files in their content simply as a static file. Just the existence of an .md file isn't as clear an indication as it might seem.

Having said that, listing active readers in debug log is a reasonable idea.

@cpitclaudel
Copy link

What about replacing "WARNING: No valid files found in content" by "WARNING: No valid files found in content (did you forget to install a language support plugin?)"

@leotrs
Copy link

leotrs commented Aug 20, 2016

There appears to be consensus on @cpitclaudel's suggestion and it looks like a pretty straightforward change.

@adrinjalali
Copy link
Contributor

What about replacing "WARNING: No valid files found in content" by "WARNING: No valid files found in content (did you forget to install a language support plugin?)"

Plus maybe a link or a hint to the part in the documentation? I have to google the error everytime, come to such a thread, and figure the problem. Specially since many people including me, think of pelican as something which handles .md files by default.

@justinmayer
Copy link
Member

@adrinjalali: This is mentioned over and over again in the docs. I'm fine with adding more descriptive error-handling, but IMHO this is not a documentation problem.

@adrinjalali
Copy link
Contributor

@justinmayer: oh yeah absolutely. I agree that the documentation couldn't be more explicit. I'm also talking about the warning message.

@justinmayer
Copy link
Member

Ah, re-reading your comment, I now understand what you meant. Apologies for the misunderstanding. 😅

@ChrisJefferson
Copy link

The problem with the docs is that I if I see "no content found", my assumption is not that I should be looking at the docs about which directories are searched, or that I've put things in the wrong directories.

How about "no restructedText content found", where we make a list of the language parsers we ran. Then I would see that it isn't even looking for markdown.

@avaris
Copy link
Member

avaris commented May 7, 2017

OK, we can elaborate the warning. How about:

WARNING: No valid files found in content for the active readers:
 | RstReader (.rst)
 | HTMLReader (.html, .htm)
...

@adrinjalali
Copy link
Contributor

adrinjalali commented May 7, 2017 via email

@justinmayer
Copy link
Member

Would anyone in this thread like to help out by implementing the suggestion @avaris proposed?

@adrinjalali
Copy link
Contributor

You mean something like this commit @justinmayer ?
adrinjalali@be4e04f

example output:

WARNING: No valid files found in content for the active readers:                                                                                                                                                   
WARNING:  | BaseReader (static)                                                                                                                                                                                    
WARNING:  | RstReader (rst)                                                                                                                                                                                        
WARNING:  | HTMLReader (htm, html)   

@justinmayer
Copy link
Member

@adrinjalali: Perhaps a single WARNING in the output, as @avaris suggested above?

@adrinjalali
Copy link
Contributor

Changed the warning, now it shows:

WARNING: No valid files found in content for the active readers:                                                                                                                                                   
  | HTMLReader (htm, html)                                                                                                                                                                                         
  | BaseReader (static)                                                                                                                                                                                            
  | RstReader (rst)

@adrinjalali
Copy link
Contributor

Although, I'd prefer this implementation, up to you.

adrinjalali/pelican@master...adrinjalali:issue89

@justinmayer justinmayer added this to the 3.8.0 milestone Aug 3, 2018
@justinmayer
Copy link
Member

Addressed via #2310. Thanks again to Adrin for the enhancement!

@terrycojones
Copy link

terrycojones commented Apr 25, 2019

I realize this issue is closed & the above has been merged. I just ran into this situation and ended up solving it by coming here and reading the thread - despite the error message and the documentation, etc. So I could have done more, for sure, but I still think the error processing / message could be clearer. I'd suggest looking at the elements of the MARKUP list and trying an import based on what's in that list. That way you could issue an error like "You have listed 'md' in your MARKUP variable but you don't have the markdown module installed." and maybe even say something like "Did you forget to run pip install markdown?"

There's a bit of a "blame-the-user" feel to this thread...

@avaris
Copy link
Member

avaris commented Apr 25, 2019

MARKUP variable? pelican doesn't use a MARKUP variable.

@terrycojones
Copy link

Ah... sorry! I guess that was something used in older versions and that has been removed. In my pelicanconf.py I have

MARKUP = ('md', 'ipynb')

If it was still a thing (or maybe it has been renamed?) I was thinking it could provide a way to avoid walking the filesystem looking for (in this case) .md files, etc. Thanks @avaris & sorry for the noise!

@avaris
Copy link
Member

avaris commented Apr 26, 2019

Yeah, there is no setting like that. Extensions to process are obtained from active readers.

BTW, I am slightly inclined to blame the user :). Even searching for "No valid files found in content" gets you the relevant information in the first hit... But, will extending warning a bit more be helpful? Something like:

> pelican content
WARNING: No valid files found in content
  | Active readers:
  |  * BaseReader (static)
  |  * HTMLReader (htm, html)
  |  * RstReader (rst)
  |
  | Inactive readers (explicitly deactivated or missing dependency):
  |  * MarkdownReader (md, markdown, mkd, mdown)
Done: Processed 0 articles, 0 drafts, 0 pages, 0 hidden pages and 0 draft pages in 0.07 seconds.

@terrycojones
Copy link

Yes, that looks good. I don't think this should be a high priority, just something that could be a bit nicer.

And, I know the temptation (re users) Most of the time I like it when there are no (other) users :-) Thanks for all the work on Pelican!

@bjinse
Copy link

bjinse commented Dec 15, 2019

I had the same issue as terrycojones. Migrated to a new machine, installed pelican, but no markdown (yet). Did a make command to generate AND publish. Poof, website content was all gone... Should have been a bit more careful.
Luckily I found this thread within a minute.
Thumbs up for explicitly stating markdown is not installed.
Maybe especially if (like in my case) markdown files are found in the content dir.

@kaddkaka
Copy link

kaddkaka commented Nov 12, 2021

Even though the documentation is extremely clear on that?

Is is not extremely clear, the quickstart guide mentions nothing about it.

One idea could be to have a checkhealth command that checks for this kind of "issue".

@avaris
Copy link
Member

avaris commented Nov 12, 2021

@kaddkaka Which quickstart are you looking at? It's mentioned right at the start:

Install Pelican (and optionally Markdown if you intend to use it) ...

@JBacc1
Copy link

JBacc1 commented Feb 8, 2022

For what it's worth, I did not find the error message particularly explicit (something mentioning markdown and/or pip install would have helped).
WARNING No valid files found in content for the active log.py:91 readers: | BaseReader (static) | HTMLReader (htm, html) | RstReader (rst)

@Axeltherabbit
Copy link

I'm just trying pelican, I didn't go deeply in the docs yet and just read a short guide with commands, I haven't wasted 30 minutes because I found this issue in 30 secs but I agree with @JBacc1 adding something "maybe you need markdown installed if you are using .md files" in the warning would not be a big issue and help

@szhorvat
Copy link

szhorvat commented Jan 5, 2024

Version 4.9.1 does not give any sort of warning that would indicate what is wrong. Previous versions did. I just wasted an hour because of this. Yes, there should be a clear warning.

@opokatech
Copy link

I've just fell into the same trap by helping someone with the pelican (helping with making a git repo, requirements.in/txt, makefile). It worked well on her machine (because she installed markdown manually and forgot about it), but not on mine, because I did not read the doc and did not know that markdown must be installed. It took several frustrating minutes to find that out.

At the end of generating process I could see only this:

Done: Processed 0 articles, 0 drafts, 0 hidden articles, 0 pages, 0 hidden pages and 0 draft pages in 0.05 seconds.

It would be very helpful, in case when literaly nothing was processed (all counters == 0), to issue some extra textual hint like
Nothing processed, see the quickstart for help: https://docs.getpelican.com/en/latest/quickstart.html

And there it is clearly visible that markdown is necessary.

@justinmayer
Copy link
Member

It appears some kind of regression has indeed occurred. Output on 4.8.0:

➤ pelican content
[19:29:13] WARNING  Watched path does not exist: ~/.virtualenvs/tempenv-17791136813a0/content/images                                                        log.py:91
           WARNING  No valid files found in content for the active readers:                                                                                         log.py:91
                      | BaseReader (static)
                      | HTMLReader (htm, html)
                      | RstReader (rst)
Done: Processed 0 articles, 0 drafts, 0 hidden articles, 0 pages, 0 hidden pages and 0 draft pages in 0.03 seconds.

Output on 4.9.1:

➤ pelican content
Done: Processed 0 articles, 0 drafts, 0 hidden articles, 0 pages, 0 hidden pages and 0 draft pages in 0.03 seconds.

This should either be addressed in a way that eliminates this persistent confusion, or we should add markdown as a standard dependency.

I know there has been resistance to the latter, but I'm guessing the vast majority of Pelican users are writing their content in Markdown, and I don't know whether it continues to make sense in 2024 to leave markdown as an optional dependency.

@justinmayer justinmayer reopened this Mar 24, 2024
@MinchinWeb
Copy link
Contributor

I don't know whether it continues to make sense in 2024 to leave markdown as an optional dependency

I'm working on a CommonMark reader, so my that case, (Python) Markdown isn't needed. (it's probably usable now, if you want to check it out https://github.com/MinchinWeb/minchin.pelican.readers.commonmark )

Part of the reason I actually pushed ahead with this is we do already install Markdown as a required (transitive) dependency, but markdown-it-py by way of rich. So I figured I'd see if I could make use of it, since it was already installed.

Anyway, just my two bits on what I'm up to in my corner of the world with Pelican.

@avaris
Copy link
Member

avaris commented Apr 19, 2024

I know there has been resistance to the latter, but I'm guessing the vast majority of Pelican users are writing their content in Markdown, and I don't know whether it continues to make sense in 2024 to leave markdown as an optional dependency.

I may have been one of those resistors, by the virtue of trying to keep things slim. But I am happy to rescind my objection and I would be fine with making markdown a required dependency.

@justinmayer
Copy link
Member

I also value the virtue of having few required dependencies. For the moment, let's address the problem by fixing the regression and adding an even clearer message regarding what's happening and what users must do to proceed.

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

No branches or pull requests